Page MenuHomeFreeBSD
Differential D20795

Control implicit PROT_MAX() using proccontrol(1) and the FreeBSD note feature bit.
ClosedPublic
Actions

Authored by kib on Jun 28 2019, 6:16 PM.
Tags
None
Referenced Files
F114942565: D20795.id59322.diff
Fri, Apr 18, 8:24 PM
Unknown Object (File)
Thu, Apr 17, 11:03 PM
Unknown Object (File)
Tue, Apr 15, 6:38 PM
Unknown Object (File)
Sun, Mar 23, 12:05 AM
Unknown Object (File)
Mar 8 2025, 5:48 AM
Unknown Object (File)
Feb 9 2025, 7:32 PM
Unknown Object (File)
Feb 9 2025, 4:31 AM
Unknown Object (File)
Jan 25 2025, 8:04 PM
View All 60 Files
Subscribers
bcr
imp

Details

Reviewers
emaste
brooks
markj
Group Reviewers
manpages
Commits
rS349610: Add implicit PROT_MAX() knob to proccontrol(1).
rS349609: Control implicit PROT_MAX() using procctl(2) and the FreeBSD note
rS349608: Use traditional 'p' local to designate td->td_proc in kern_mmap.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 25134

Event Timeline

kib created this revision.Jun 28 2019, 6:16 PM
Herald added a reviewer: manpages. · View Herald TranscriptJun 28 2019, 6:16 PM
Herald added a subscriber: imp. · View Herald Transcript
Harbormaster completed remote builds in B25109: Diff 59163.Jun 28 2019, 6:16 PM
emaste added a comment.Jun 28 2019, 6:28 PM

Overall looks reasonable, a few small alpha ordering nits.

sys/compat/freebsd32/freebsd32_misc.c
3338

alpha order

sys/vm/vm_mmap.c
60

can this go in alpha order, other misordered headers notwithstanding?

usr.bin/proccontrol/proccontrol.c
76

alpha order

bcr added a subscriber: bcr.Jun 28 2019, 6:42 PM

Suggested rewording of a sentence.

lib/libc/sys/procctl.2
129

The image flag might still prevent the enablement.

kib marked 4 inline comments as done.Jun 28 2019, 7:31 PM
kib added inline comments.
sys/vm/vm_mmap.c
60

Not sure what would be the right place then. Moved before filedesc.h.

markj added inline comments.Jun 30 2019, 6:08 PM
lib/libc/sys/procctl.2
119

and .Xr mprotect 2

128

It would be nice to reference the sysctl here, "disabled system-wide by vm.imply_prot_max"

131

Should be "be disabled."

133

Extra _ between PROT and MAX.

136

"Use the system-wide ..."

kib updated this revision to Diff 59221.Jun 30 2019, 7:06 PM
kib marked an inline comment as done.

Man page update and minor code rearrangement.

Harbormaster completed remote builds in B25134: Diff 59221.Jun 30 2019, 7:06 PM
markj accepted this revision.Jul 2 2019, 3:00 PM
This revision is now accepted and ready to land.Jul 2 2019, 3:00 PM
emaste accepted this revision.Jul 2 2019, 3:08 PM
brooks added inline comments.Jul 2 2019, 4:23 PM
lib/libc/sys/procctl.2
119

We don't imply PROT_MAX on mprotect at this time. One could make a case for it, but we're never tested it and it's not as clear a case IMO.

sys/sys/syscallsubr.h
178

It seems a little odd to me that this is here and not in some sys/vm/* header, but don't have a strong opinion.

Closed by commit rS349608: Use traditional 'p' local to designate td->td_proc in kern_mmap. (authored by kib). · Explain WhyJul 2 2019, 7:01 PM
This revision was automatically updated to reflect the committed changes.
kib added a commit: rS349608: Use traditional 'p' local to designate td->td_proc in kern_mmap..
kib added a commit: rS349609: Control implicit PROT_MAX() using procctl(2) and the FreeBSD note.Jul 2 2019, 7:07 PM
kib added a commit: rS349610: Add implicit PROT_MAX() knob to proccontrol(1)..Jul 2 2019, 7:12 PM

Revision Contents
Changeset List

PathSize
lib/
libc/
sys/
48 lines
sys/
compat/
freebsd32/
3 lines
kern/
57 lines
sys/
1 line
2 lines
7 lines
1 line
vm/
29 lines
usr.bin/
proccontrol/
3 lines
30 lines

Diff 59221

View Options

lib/libc/sys/procctl.2

Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Lines
* *
* The environment variable vfs.root.mountfrom.options is a comma delimited * The environment variable vfs.root.mountfrom.options is a comma delimited
* set of string mount options. These mount options must be parseable * set of string mount options. These mount options must be parseable
* by nmount() in the kernel. * by nmount() in the kernel.
*/ */
static int parse_mount(char **); static int parse_mount(char **);
static struct mntarg *parse_mountroot_options(struct mntarg *, const char *); static struct mntarg *parse_mountroot_options(struct mntarg *, const char *);
static int vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev);
/* /*
* The vnode of the system's root (/ in the filesystem, without chroot * The vnode of the system's root (/ in the filesystem, without chroot
* active.) * active.)
*/ */
struct vnode *rootvnode; struct vnode *rootvnode;
/* /*
▲ Show 20 Lines • Show All 579 Lines • ▼ Show 20 Linesparse_mount_dev_present(const char *dev)
struct nameidata nd; struct nameidata nd;
int error; int error;
NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread); NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread);
error = namei(&nd); error = namei(&nd);
if (!error) if (!error)
vput(nd.ni_vp); vput(nd.ni_vp);
NDFREE(&nd, NDF_ONLY_PNBUF); NDFREE(&nd, NDF_ONLY_PNBUF);
if (error != 0)
printf("%s: %s not present, error %d\n", __func__, dev, error);
marcelUnsubmitted
Done
Inline Actions

This looks like it's more for debugging than for an end user. Maybe put this under boot verbose? The reason is that it's not an error until it's an error, meaning that if we're going to wait for the device when it's not there yet, then printing errors is confusing. Putting it under boot verbose is a nice way for people to get insight into why a boot may be stalled...

marcel: This looks like it's more for debugging than for an end user. Maybe put this under boot verbose?
return (error != 0) ? 0 : 1; return (error != 0) ? 0 : 1;
} }
traszAuthorUnsubmitted
Not Done
Inline Actions

Actually, it was a debugging printf that snuck in. Removed.

trasz: Actually, it was a debugging printf that snuck in. Removed.
#define ERRMSGL 255 #define ERRMSGL 255
static int static int
parse_mount(char **conf) parse_mount(char **conf)
{ {
char *errmsg; char *errmsg;
struct mntarg *ma; struct mntarg *ma;
char *dev, *fs, *opts, *tok; char *dev, *fs, *opts, *tok;
int delay, error, timeout; int error;
error = parse_token(conf, &tok); error = parse_token(conf, &tok);
if (error) if (error)
return (error); return (error);
fs = tok; fs = tok;
error = parse_skipto(&tok, ':'); error = parse_skipto(&tok, ':');
if (error) { if (error) {
free(fs, M_TEMP); free(fs, M_TEMP);
Show All 20 Linesparse_mount(char **conf)
errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO); errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO);
if (vfs_byname(fs) == NULL) { if (vfs_byname(fs) == NULL) {
strlcpy(errmsg, "unknown file system", ERRMSGL); strlcpy(errmsg, "unknown file system", ERRMSGL);
error = ENOENT; error = ENOENT;
goto out; goto out;
} }
if (strcmp(fs, "zfs") != 0 && strstr(fs, "nfs") == NULL && error = vfs_mountroot_wait_if_neccessary(fs, dev);
dev[0] != '\0' && !parse_mount_dev_present(dev)) { if (error != 0)
printf("mountroot: waiting for device %s ...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0) {
error = ENODEV;
goto out; goto out;
}
}
ma = NULL; ma = NULL;
ma = mount_arg(ma, "fstype", fs, -1); ma = mount_arg(ma, "fstype", fs, -1);
ma = mount_arg(ma, "fspath", "/", -1); ma = mount_arg(ma, "fspath", "/", -1);
ma = mount_arg(ma, "from", dev, -1); ma = mount_arg(ma, "from", dev, -1);
ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL); ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL);
ma = mount_arg(ma, "ro", NULL, 0); ma = mount_arg(ma, "ro", NULL, 0);
ma = parse_mountroot_options(ma, opts); ma = parse_mountroot_options(ma, opts);
▲ Show 20 Lines • Show All 191 Lines • ▼ Show 20 Lines if (ppsratecheck(&lastfail, &curfail, 1)) {
printf(" %s", h->who); printf(" %s", h->who);
printf("\n"); printf("\n");
} }
msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold", msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold",
hz); hz);
} }
} }
static int
vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev)
kibUnsubmitted
Not Done
Inline Actions

Please come with much shorter name for the function.

kib: Please come with much shorter name for the function.
{
int delay, timeout;
/*
* In case of ZFS and NFS we don't have a way to wait for
* specific device.
*/
if (strcmp(fs, "zfs") == 0 || strstr(fs, "nfs") != NULL ||
dev[0] == '\0') {
vfs_mountroot_wait();
return (0);
}
/*
* Otherwise, no point in waiting if the device is already there.
* Note that we must wait for GEOM to finish reconfiguring itself,
* eg for geom_part(4) to finish tasting.
*/
DROP_GIANT();
g_waitidle();
kibUnsubmitted
Not Done
Inline Actions

I think this is not needed, and probably even dangerous, for the nfs case. I have no better suggestion than ugly strstr("nfs") hack, but it is still much better than preventing nfs mount due to the issues with the local storage subsystem.

kib: I think this is not needed, and probably even dangerous, for the nfs case. I have no better…
traszAuthorUnsubmitted
Not Done
Inline Actions

For the non-NFS case it's definitely needed, because without it, partitions don't get created before we check for the device presence, and we end up doing the full wait in all cases, defying the purpose of this patch.

For the NFS case, this code is not reached - just like for ZFS, we call vfs_mountroot_wait() and return few lines earlier.

trasz: For the non-NFS case it's definitely needed, because without it, partitions don't get created…
PICKUP_GIANT();
if (parse_mount_dev_present(dev))
return (0);
/*
* No luck. Let's wait. This code looks weird, but it's that way
* to behave exactly as it used to work before.
*/
vfs_mountroot_wait();
printf("mountroot: waiting for device %s...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0)
return (ENODEV);
kibUnsubmitted
Not Done
Inline Actions

This blank line is not needed.

kib: This blank line is not needed.
return (0);
}
void void
vfs_mountroot(void) vfs_mountroot(void)
{ {
struct mount *mp; struct mount *mp;
struct sbuf *sb; struct sbuf *sb;
struct thread *td; struct thread *td;
time_t timebase; time_t timebase;
int error; int error;
td = curthread; td = curthread;
vfs_mountroot_wait();
sb = sbuf_new_auto(); sb = sbuf_new_auto();
vfs_mountroot_conf0(sb); vfs_mountroot_conf0(sb);
sbuf_finish(sb); sbuf_finish(sb);
error = vfs_mountroot_devfs(td, &mp); error = vfs_mountroot_devfs(td, &mp);
while (!error) { while (!error) {
error = vfs_mountroot_parse(sb, mp); error = vfs_mountroot_parse(sb, mp);
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines
View Options

sys/compat/freebsd32/freebsd32_misc.c

Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Lines
* *
* The environment variable vfs.root.mountfrom.options is a comma delimited * The environment variable vfs.root.mountfrom.options is a comma delimited
* set of string mount options. These mount options must be parseable * set of string mount options. These mount options must be parseable
* by nmount() in the kernel. * by nmount() in the kernel.
*/ */
static int parse_mount(char **); static int parse_mount(char **);
static struct mntarg *parse_mountroot_options(struct mntarg *, const char *); static struct mntarg *parse_mountroot_options(struct mntarg *, const char *);
static int vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev);
/* /*
* The vnode of the system's root (/ in the filesystem, without chroot * The vnode of the system's root (/ in the filesystem, without chroot
* active.) * active.)
*/ */
struct vnode *rootvnode; struct vnode *rootvnode;
/* /*
▲ Show 20 Lines • Show All 579 Lines • ▼ Show 20 Linesparse_mount_dev_present(const char *dev)
struct nameidata nd; struct nameidata nd;
int error; int error;
NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread); NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread);
error = namei(&nd); error = namei(&nd);
if (!error) if (!error)
vput(nd.ni_vp); vput(nd.ni_vp);
NDFREE(&nd, NDF_ONLY_PNBUF); NDFREE(&nd, NDF_ONLY_PNBUF);
if (error != 0)
printf("%s: %s not present, error %d\n", __func__, dev, error);
marcelUnsubmitted
Done
Inline Actions

This looks like it's more for debugging than for an end user. Maybe put this under boot verbose? The reason is that it's not an error until it's an error, meaning that if we're going to wait for the device when it's not there yet, then printing errors is confusing. Putting it under boot verbose is a nice way for people to get insight into why a boot may be stalled...

marcel: This looks like it's more for debugging than for an end user. Maybe put this under boot verbose?
return (error != 0) ? 0 : 1; return (error != 0) ? 0 : 1;
} }
traszAuthorUnsubmitted
Not Done
Inline Actions

Actually, it was a debugging printf that snuck in. Removed.

trasz: Actually, it was a debugging printf that snuck in. Removed.
#define ERRMSGL 255 #define ERRMSGL 255
static int static int
parse_mount(char **conf) parse_mount(char **conf)
{ {
char *errmsg; char *errmsg;
struct mntarg *ma; struct mntarg *ma;
char *dev, *fs, *opts, *tok; char *dev, *fs, *opts, *tok;
int delay, error, timeout; int error;
error = parse_token(conf, &tok); error = parse_token(conf, &tok);
if (error) if (error)
return (error); return (error);
fs = tok; fs = tok;
error = parse_skipto(&tok, ':'); error = parse_skipto(&tok, ':');
if (error) { if (error) {
free(fs, M_TEMP); free(fs, M_TEMP);
Show All 20 Linesparse_mount(char **conf)
errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO); errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO);
if (vfs_byname(fs) == NULL) { if (vfs_byname(fs) == NULL) {
strlcpy(errmsg, "unknown file system", ERRMSGL); strlcpy(errmsg, "unknown file system", ERRMSGL);
error = ENOENT; error = ENOENT;
goto out; goto out;
} }
if (strcmp(fs, "zfs") != 0 && strstr(fs, "nfs") == NULL && error = vfs_mountroot_wait_if_neccessary(fs, dev);
dev[0] != '\0' && !parse_mount_dev_present(dev)) { if (error != 0)
printf("mountroot: waiting for device %s ...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0) {
error = ENODEV;
goto out; goto out;
}
}
ma = NULL; ma = NULL;
ma = mount_arg(ma, "fstype", fs, -1); ma = mount_arg(ma, "fstype", fs, -1);
ma = mount_arg(ma, "fspath", "/", -1); ma = mount_arg(ma, "fspath", "/", -1);
ma = mount_arg(ma, "from", dev, -1); ma = mount_arg(ma, "from", dev, -1);
ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL); ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL);
ma = mount_arg(ma, "ro", NULL, 0); ma = mount_arg(ma, "ro", NULL, 0);
ma = parse_mountroot_options(ma, opts); ma = parse_mountroot_options(ma, opts);
▲ Show 20 Lines • Show All 191 Lines • ▼ Show 20 Lines if (ppsratecheck(&lastfail, &curfail, 1)) {
printf(" %s", h->who); printf(" %s", h->who);
printf("\n"); printf("\n");
} }
msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold", msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold",
hz); hz);
} }
} }
static int
vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev)
kibUnsubmitted
Not Done
Inline Actions

Please come with much shorter name for the function.

kib: Please come with much shorter name for the function.
{
int delay, timeout;
/*
* In case of ZFS and NFS we don't have a way to wait for
* specific device.
*/
if (strcmp(fs, "zfs") == 0 || strstr(fs, "nfs") != NULL ||
dev[0] == '\0') {
vfs_mountroot_wait();
return (0);
}
/*
* Otherwise, no point in waiting if the device is already there.
* Note that we must wait for GEOM to finish reconfiguring itself,
* eg for geom_part(4) to finish tasting.
*/
DROP_GIANT();
g_waitidle();
kibUnsubmitted
Not Done
Inline Actions

I think this is not needed, and probably even dangerous, for the nfs case. I have no better suggestion than ugly strstr("nfs") hack, but it is still much better than preventing nfs mount due to the issues with the local storage subsystem.

kib: I think this is not needed, and probably even dangerous, for the nfs case. I have no better…
traszAuthorUnsubmitted
Not Done
Inline Actions

For the non-NFS case it's definitely needed, because without it, partitions don't get created before we check for the device presence, and we end up doing the full wait in all cases, defying the purpose of this patch.

For the NFS case, this code is not reached - just like for ZFS, we call vfs_mountroot_wait() and return few lines earlier.

trasz: For the non-NFS case it's definitely needed, because without it, partitions don't get created…
PICKUP_GIANT();
if (parse_mount_dev_present(dev))
return (0);
/*
* No luck. Let's wait. This code looks weird, but it's that way
* to behave exactly as it used to work before.
*/
vfs_mountroot_wait();
printf("mountroot: waiting for device %s...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0)
return (ENODEV);
kibUnsubmitted
Not Done
Inline Actions

This blank line is not needed.

kib: This blank line is not needed.
return (0);
}
void void
vfs_mountroot(void) vfs_mountroot(void)
{ {
struct mount *mp; struct mount *mp;
struct sbuf *sb; struct sbuf *sb;
struct thread *td; struct thread *td;
time_t timebase; time_t timebase;
int error; int error;
td = curthread; td = curthread;
vfs_mountroot_wait();
sb = sbuf_new_auto(); sb = sbuf_new_auto();
vfs_mountroot_conf0(sb); vfs_mountroot_conf0(sb);
sbuf_finish(sb); sbuf_finish(sb);
error = vfs_mountroot_devfs(td, &mp); error = vfs_mountroot_devfs(td, &mp);
while (!error) { while (!error) {
error = vfs_mountroot_parse(sb, mp); error = vfs_mountroot_parse(sb, mp);
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines
View Options

sys/kern/kern_procctl.c

Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Lines
* *
* The environment variable vfs.root.mountfrom.options is a comma delimited * The environment variable vfs.root.mountfrom.options is a comma delimited
* set of string mount options. These mount options must be parseable * set of string mount options. These mount options must be parseable
* by nmount() in the kernel. * by nmount() in the kernel.
*/ */
static int parse_mount(char **); static int parse_mount(char **);
static struct mntarg *parse_mountroot_options(struct mntarg *, const char *); static struct mntarg *parse_mountroot_options(struct mntarg *, const char *);
static int vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev);
/* /*
* The vnode of the system's root (/ in the filesystem, without chroot * The vnode of the system's root (/ in the filesystem, without chroot
* active.) * active.)
*/ */
struct vnode *rootvnode; struct vnode *rootvnode;
/* /*
▲ Show 20 Lines • Show All 579 Lines • ▼ Show 20 Linesparse_mount_dev_present(const char *dev)
struct nameidata nd; struct nameidata nd;
int error; int error;
NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread); NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread);
error = namei(&nd); error = namei(&nd);
if (!error) if (!error)
vput(nd.ni_vp); vput(nd.ni_vp);
NDFREE(&nd, NDF_ONLY_PNBUF); NDFREE(&nd, NDF_ONLY_PNBUF);
if (error != 0)
printf("%s: %s not present, error %d\n", __func__, dev, error);
marcelUnsubmitted
Done
Inline Actions

This looks like it's more for debugging than for an end user. Maybe put this under boot verbose? The reason is that it's not an error until it's an error, meaning that if we're going to wait for the device when it's not there yet, then printing errors is confusing. Putting it under boot verbose is a nice way for people to get insight into why a boot may be stalled...

marcel: This looks like it's more for debugging than for an end user. Maybe put this under boot verbose?
return (error != 0) ? 0 : 1; return (error != 0) ? 0 : 1;
} }
traszAuthorUnsubmitted
Not Done
Inline Actions

Actually, it was a debugging printf that snuck in. Removed.

trasz: Actually, it was a debugging printf that snuck in. Removed.
#define ERRMSGL 255 #define ERRMSGL 255
static int static int
parse_mount(char **conf) parse_mount(char **conf)
{ {
char *errmsg; char *errmsg;
struct mntarg *ma; struct mntarg *ma;
char *dev, *fs, *opts, *tok; char *dev, *fs, *opts, *tok;
int delay, error, timeout; int error;
error = parse_token(conf, &tok); error = parse_token(conf, &tok);
if (error) if (error)
return (error); return (error);
fs = tok; fs = tok;
error = parse_skipto(&tok, ':'); error = parse_skipto(&tok, ':');
if (error) { if (error) {
free(fs, M_TEMP); free(fs, M_TEMP);
Show All 20 Linesparse_mount(char **conf)
errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO); errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO);
if (vfs_byname(fs) == NULL) { if (vfs_byname(fs) == NULL) {
strlcpy(errmsg, "unknown file system", ERRMSGL); strlcpy(errmsg, "unknown file system", ERRMSGL);
error = ENOENT; error = ENOENT;
goto out; goto out;
} }
if (strcmp(fs, "zfs") != 0 && strstr(fs, "nfs") == NULL && error = vfs_mountroot_wait_if_neccessary(fs, dev);
dev[0] != '\0' && !parse_mount_dev_present(dev)) { if (error != 0)
printf("mountroot: waiting for device %s ...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0) {
error = ENODEV;
goto out; goto out;
}
}
ma = NULL; ma = NULL;
ma = mount_arg(ma, "fstype", fs, -1); ma = mount_arg(ma, "fstype", fs, -1);
ma = mount_arg(ma, "fspath", "/", -1); ma = mount_arg(ma, "fspath", "/", -1);
ma = mount_arg(ma, "from", dev, -1); ma = mount_arg(ma, "from", dev, -1);
ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL); ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL);
ma = mount_arg(ma, "ro", NULL, 0); ma = mount_arg(ma, "ro", NULL, 0);
ma = parse_mountroot_options(ma, opts); ma = parse_mountroot_options(ma, opts);
▲ Show 20 Lines • Show All 191 Lines • ▼ Show 20 Lines if (ppsratecheck(&lastfail, &curfail, 1)) {
printf(" %s", h->who); printf(" %s", h->who);
printf("\n"); printf("\n");
} }
msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold", msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold",
hz); hz);
} }
} }
static int
vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev)
kibUnsubmitted
Not Done
Inline Actions

Please come with much shorter name for the function.

kib: Please come with much shorter name for the function.
{
int delay, timeout;
/*
* In case of ZFS and NFS we don't have a way to wait for
* specific device.
*/
if (strcmp(fs, "zfs") == 0 || strstr(fs, "nfs") != NULL ||
dev[0] == '\0') {
vfs_mountroot_wait();
return (0);
}
/*
* Otherwise, no point in waiting if the device is already there.
* Note that we must wait for GEOM to finish reconfiguring itself,
* eg for geom_part(4) to finish tasting.
*/
DROP_GIANT();
g_waitidle();
kibUnsubmitted
Not Done
Inline Actions

I think this is not needed, and probably even dangerous, for the nfs case. I have no better suggestion than ugly strstr("nfs") hack, but it is still much better than preventing nfs mount due to the issues with the local storage subsystem.

kib: I think this is not needed, and probably even dangerous, for the nfs case. I have no better…
traszAuthorUnsubmitted
Not Done
Inline Actions

For the non-NFS case it's definitely needed, because without it, partitions don't get created before we check for the device presence, and we end up doing the full wait in all cases, defying the purpose of this patch.

For the NFS case, this code is not reached - just like for ZFS, we call vfs_mountroot_wait() and return few lines earlier.

trasz: For the non-NFS case it's definitely needed, because without it, partitions don't get created…
PICKUP_GIANT();
if (parse_mount_dev_present(dev))
return (0);
/*
* No luck. Let's wait. This code looks weird, but it's that way
* to behave exactly as it used to work before.
*/
vfs_mountroot_wait();
printf("mountroot: waiting for device %s...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0)
return (ENODEV);
kibUnsubmitted
Not Done
Inline Actions

This blank line is not needed.

kib: This blank line is not needed.
return (0);
}
void void
vfs_mountroot(void) vfs_mountroot(void)
{ {
struct mount *mp; struct mount *mp;
struct sbuf *sb; struct sbuf *sb;
struct thread *td; struct thread *td;
time_t timebase; time_t timebase;
int error; int error;
td = curthread; td = curthread;
vfs_mountroot_wait();
sb = sbuf_new_auto(); sb = sbuf_new_auto();
vfs_mountroot_conf0(sb); vfs_mountroot_conf0(sb);
sbuf_finish(sb); sbuf_finish(sb);
error = vfs_mountroot_devfs(td, &mp); error = vfs_mountroot_devfs(td, &mp);
while (!error) { while (!error) {
error = vfs_mountroot_parse(sb, mp); error = vfs_mountroot_parse(sb, mp);
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines
View Options

sys/sys/elf_common.h

Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Lines
* *
* The environment variable vfs.root.mountfrom.options is a comma delimited * The environment variable vfs.root.mountfrom.options is a comma delimited
* set of string mount options. These mount options must be parseable * set of string mount options. These mount options must be parseable
* by nmount() in the kernel. * by nmount() in the kernel.
*/ */
static int parse_mount(char **); static int parse_mount(char **);
static struct mntarg *parse_mountroot_options(struct mntarg *, const char *); static struct mntarg *parse_mountroot_options(struct mntarg *, const char *);
static int vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev);
/* /*
* The vnode of the system's root (/ in the filesystem, without chroot * The vnode of the system's root (/ in the filesystem, without chroot
* active.) * active.)
*/ */
struct vnode *rootvnode; struct vnode *rootvnode;
/* /*
▲ Show 20 Lines • Show All 579 Lines • ▼ Show 20 Linesparse_mount_dev_present(const char *dev)
struct nameidata nd; struct nameidata nd;
int error; int error;
NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread); NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, dev, curthread);
error = namei(&nd); error = namei(&nd);
if (!error) if (!error)
vput(nd.ni_vp); vput(nd.ni_vp);
NDFREE(&nd, NDF_ONLY_PNBUF); NDFREE(&nd, NDF_ONLY_PNBUF);
if (error != 0)
printf("%s: %s not present, error %d\n", __func__, dev, error);
marcelUnsubmitted
Done
Inline Actions

This looks like it's more for debugging than for an end user. Maybe put this under boot verbose? The reason is that it's not an error until it's an error, meaning that if we're going to wait for the device when it's not there yet, then printing errors is confusing. Putting it under boot verbose is a nice way for people to get insight into why a boot may be stalled...

marcel: This looks like it's more for debugging than for an end user. Maybe put this under boot verbose?
return (error != 0) ? 0 : 1; return (error != 0) ? 0 : 1;
} }
traszAuthorUnsubmitted
Not Done
Inline Actions

Actually, it was a debugging printf that snuck in. Removed.

trasz: Actually, it was a debugging printf that snuck in. Removed.
#define ERRMSGL 255 #define ERRMSGL 255
static int static int
parse_mount(char **conf) parse_mount(char **conf)
{ {
char *errmsg; char *errmsg;
struct mntarg *ma; struct mntarg *ma;
char *dev, *fs, *opts, *tok; char *dev, *fs, *opts, *tok;
int delay, error, timeout; int error;
error = parse_token(conf, &tok); error = parse_token(conf, &tok);
if (error) if (error)
return (error); return (error);
fs = tok; fs = tok;
error = parse_skipto(&tok, ':'); error = parse_skipto(&tok, ':');
if (error) { if (error) {
free(fs, M_TEMP); free(fs, M_TEMP);
Show All 20 Linesparse_mount(char **conf)
errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO); errmsg = malloc(ERRMSGL, M_TEMP, M_WAITOK | M_ZERO);
if (vfs_byname(fs) == NULL) { if (vfs_byname(fs) == NULL) {
strlcpy(errmsg, "unknown file system", ERRMSGL); strlcpy(errmsg, "unknown file system", ERRMSGL);
error = ENOENT; error = ENOENT;
goto out; goto out;
} }
if (strcmp(fs, "zfs") != 0 && strstr(fs, "nfs") == NULL && error = vfs_mountroot_wait_if_neccessary(fs, dev);
dev[0] != '\0' && !parse_mount_dev_present(dev)) { if (error != 0)
printf("mountroot: waiting for device %s ...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0) {
error = ENODEV;
goto out; goto out;
}
}
ma = NULL; ma = NULL;
ma = mount_arg(ma, "fstype", fs, -1); ma = mount_arg(ma, "fstype", fs, -1);
ma = mount_arg(ma, "fspath", "/", -1); ma = mount_arg(ma, "fspath", "/", -1);
ma = mount_arg(ma, "from", dev, -1); ma = mount_arg(ma, "from", dev, -1);
ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL); ma = mount_arg(ma, "errmsg", errmsg, ERRMSGL);
ma = mount_arg(ma, "ro", NULL, 0); ma = mount_arg(ma, "ro", NULL, 0);
ma = parse_mountroot_options(ma, opts); ma = parse_mountroot_options(ma, opts);
▲ Show 20 Lines • Show All 191 Lines • ▼ Show 20 Lines if (ppsratecheck(&lastfail, &curfail, 1)) {
printf(" %s", h->who); printf(" %s", h->who);
printf("\n"); printf("\n");
} }
msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold", msleep(&root_holds, &root_holds_mtx, PZERO | PDROP, "roothold",
hz); hz);
} }
} }
static int
vfs_mountroot_wait_if_neccessary(const char *fs, const char *dev)
kibUnsubmitted
Not Done
Inline Actions

Please come with much shorter name for the function.

kib: Please come with much shorter name for the function.
{
int delay, timeout;
/*
* In case of ZFS and NFS we don't have a way to wait for
* specific device.
*/
if (strcmp(fs, "zfs") == 0 || strstr(fs, "nfs") != NULL ||
dev[0] == '\0') {
vfs_mountroot_wait();
return (0);
}
/*
* Otherwise, no point in waiting if the device is already there.
* Note that we must wait for GEOM to finish reconfiguring itself,
* eg for geom_part(4) to finish tasting.
*/
DROP_GIANT();
g_waitidle();
kibUnsubmitted
Not Done
Inline Actions

I think this is not needed, and probably even dangerous, for the nfs case. I have no better suggestion than ugly strstr("nfs") hack, but it is still much better than preventing nfs mount due to the issues with the local storage subsystem.

kib: I think this is not needed, and probably even dangerous, for the nfs case. I have no better…
traszAuthorUnsubmitted
Not Done
Inline Actions

For the non-NFS case it's definitely needed, because without it, partitions don't get created before we check for the device presence, and we end up doing the full wait in all cases, defying the purpose of this patch.

For the NFS case, this code is not reached - just like for ZFS, we call vfs_mountroot_wait() and return few lines earlier.

trasz: For the non-NFS case it's definitely needed, because without it, partitions don't get created…
PICKUP_GIANT();
if (parse_mount_dev_present(dev))
return (0);
/*
* No luck. Let's wait. This code looks weird, but it's that way
* to behave exactly as it used to work before.
*/
vfs_mountroot_wait();
printf("mountroot: waiting for device %s...\n", dev);
delay = hz / 10;
timeout = root_mount_timeout * hz;
do {
pause("rmdev", delay);
timeout -= delay;
} while (timeout > 0 && !parse_mount_dev_present(dev));
if (timeout <= 0)
return (ENODEV);
kibUnsubmitted
Not Done
Inline Actions

This blank line is not needed.

kib: This blank line is not needed.
return (0);
}
void void
vfs_mountroot(void) vfs_mountroot(void)
{ {
struct mount *mp; struct mount *mp;
struct sbuf *sb; struct sbuf *sb;
struct thread *td; struct thread *td;
time_t timebase; time_t timebase;
int error; int error;
td = curthread; td = curthread;
vfs_mountroot_wait();
sb = sbuf_new_auto(); sb = sbuf_new_auto();
vfs_mountroot_conf0(sb); vfs_mountroot_conf0(sb);
sbuf_finish(sb); sbuf_finish(sb);
error = vfs_mountroot_devfs(td, &mp); error = vfs_mountroot_devfs(td, &mp);
while (!error) { while (!error) {
error = vfs_mountroot_parse(sb, mp); error = vfs_mountroot_parse(sb, mp);
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines
View Options

sys/sys/proc.h

Loading...
View Options

sys/sys/procctl.h

Loading...
View Options

sys/sys/syscallsubr.h

Loading...
View Options

sys/vm/vm_mmap.c

Loading...
View Options

usr.bin/proccontrol/proccontrol.1

Loading...
View Options

usr.bin/proccontrol/proccontrol.c

Loading...