Bug 262471

Summary: jail utility crashes when supplied -m and vnet parameter
Product: Base System Reporter: jcaplan
Component: binAssignee: Jamie Gritton <jamie>
Status: Closed FIXED    
Severity: Affects Only Me CC: jamie, markj
Priority: ---    
Version: 13.0-STABLE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Properly detect value-less parameters in rdtun_params none

Description jcaplan 2022-03-10 17:20:26 UTC
Overview
--------

Jail utility can crash in jailparam_free(), not sure if it's an issue with jail passing bad inputs or something in libjail and jailparam_free()

Steps to Reproduce
------------------

# jail -c name=test persist vnet
# jail -m name=jail vnet vnet.interface=vmx0

Actual Results
--------------
Segmentation fault (core dumped)


Expected Results
----------------
Doesn't core

Build Date & Hardware
---------------------
FreeBSD freebsd 13.0-RELEASE FreeBSD 13.0-RELEASE #7 releng/13.0-n244733-ea31abc261f-dirty: Wed Jul 21 14:53:10 UTC 2021     jcaplan@freebsd:/usr/obj/usr/src/amd64.amd64/sys/DEBUG  amd64
Comment 1 Jamie Gritton freebsd_committer freebsd_triage 2022-03-24 17:53:32 UTC
I see the problem.  In the check for non-changeable parameters, it's trying to dereference the value of vnet.  There's code to catch boolean parameters with implicit values, but it misses vnet which isn't a true boolean.

Working on making a hideous-looking if statement even worse...
Comment 2 Jamie Gritton freebsd_committer freebsd_triage 2022-03-24 20:25:00 UTC
Created attachment 232681 [details]
Properly detect value-less parameters in rdtun_params

This seems to do the trick, as well as making the code a touch more readable.  It now not only works for boolean parameters, but "jailsys" parameters like vnet, and string parameters like path.  Any of these might have no value set, and would have crashed there.

This is a long-standing bug, with code from the beginning of the second-gen jails.  I guess not many people run things the way you did, though it's perfectly valid.
Comment 3 Jamie Gritton freebsd_committer freebsd_triage 2022-03-26 02:17:58 UTC
Fixed in 490b09f240065d7ef61f68ec1bf134d729cfad28.  Oops, I forgot to mention this bug in the commit - I'll try to remember on an MFC.
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-03-28 23:41:41 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c1576434e9cf9c48b4d3975717c9f6cc6427cfd9

commit c1576434e9cf9c48b4d3975717c9f6cc6427cfd9
Author:     Jamie Gritton <jamie@FreeBSD.org>
AuthorDate: 2022-03-26 02:16:51 +0000
Commit:     Jamie Gritton <jamie@FreeBSD.org>
CommitDate: 2022-03-28 23:39:54 +0000

    mfc jail: handle jailsys parameters in modification permission test

    Avoid a null dereference when a value-less jailsys parameter is passed
    to "jail -m".  There was already code to handle boolean parameters,
    but in reality any parameter could be passed without a value.

    PR:             262471
    Reported by:    jcaplan at blackberry.com

    (cherry picked from commit 8f1543785f77086494c73310ba8f5d09b61ff7eb)

 usr.sbin/jail/jail.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-03-28 23:41:42 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f059a2c832f8cff0d3c0db142a3216b13b4c0480

commit f059a2c832f8cff0d3c0db142a3216b13b4c0480
Author:     Jamie Gritton <jamie@FreeBSD.org>
AuthorDate: 2022-03-26 02:16:51 +0000
Commit:     Jamie Gritton <jamie@FreeBSD.org>
CommitDate: 2022-03-28 23:41:12 +0000

    mfc jail: handle jailsys parameters in modification permission test

    Avoid a null dereference when a value-less jailsys parameter is passed
    to "jail -m".  There was already code to handle boolean parameters,
    but in reality any parameter could be passed without a value.

    PR:             262471
    Reported by:    jcaplan at blackberry.com

    (cherry picked from commit 8f1543785f77086494c73310ba8f5d09b61ff7eb)

 usr.sbin/jail/jail.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)