Messages in this thread | | | Date | Tue, 20 Nov 2012 12:54:59 -0800 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH v2] devtmpfs: mount with noexec and nosuid |
| |
On Tue, Nov 20, 2012 at 12:42:38PM -0800, Kees Cook wrote: > Since devtmpfs is writable, make the default noexec,nosuid as well. This > protects from the case of a privileged process having an arbitrary file > write flaw and an argumentless arbitrary execution (i.e. it would lack > the ability to run "mount -o remount,exec,suid /dev"). > > Cc: ellyjones@chromium.org > Cc: Kay Sievers <kay@vrfy.org> > Cc: Roland Eggner <edvx1@systemanalysen.net> > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > v2: > - use CONFIG_DEVTMPFS_SAFE to wrap the logic.
That's better, thanks.
One tiny bikeshead request though:
> --- a/drivers/base/devtmpfs.c > +++ b/drivers/base/devtmpfs.c > @@ -340,6 +340,10 @@ static int handle_remove(const char *nodename, struct device *dev) > int devtmpfs_mount(const char *mntdir) > { > int err; > + int mflags = MS_SILENT; > +#ifdef CONFIG_DEVTMPFS_SAFE > + mflags |= MS_NOEXEC | MS_NOSUID; > +#endif
You duplicate this in two places, which makes the c code harder to read. How about, at the top of the file, you do:
#ifdef CONFIG_DEVTMPFS_SAFE #define DEVTMPFS_MFLAGS MS_SILENT | MS_NOEXEC | MS_NOSUID #else #define DEVTMPFS_MFLAGS MS_SILENT #endif
And then just use DEVTMPFS_MFLAGS in both places?
That should make the patch smaller, which is always nice :)
thanks,
greg k-h
| |