On Thu, Jul 16, 2020 at 04:39:14PM +0200, Mickaël Salaün wrote: > > On 15/07/2020 22:37, Kees Cook wrote: > > On Tue, Jul 14, 2020 at 08:16:36PM +0200, Mickaël Salaün wrote: > >> @@ -2849,7 +2855,7 @@ static int may_open(const struct path *path, int acc_mode, int flag) > >> case S_IFLNK: > >> return -ELOOP; > >> case S_IFDIR: > >> - if (acc_mode & (MAY_WRITE | MAY_EXEC)) > >> + if (acc_mode & (MAY_WRITE | MAY_EXEC | MAY_OPENEXEC)) > >> return -EISDIR; > >> break; > > > > (I need to figure out where "open for reading" rejects S_IFDIR, since > > it's clearly not here...) Doesn't it come from generic_read_dir() in fs/libfs.c? > > > >> case S_IFBLK: > >> @@ -2859,13 +2865,26 @@ static int may_open(const struct path *path, int acc_mode, int flag) > >> fallthrough; > >> case S_IFIFO: > >> case S_IFSOCK: > >> - if (acc_mode & MAY_EXEC) > >> + if (acc_mode & (MAY_EXEC | MAY_OPENEXEC)) > >> return -EACCES; > >> flag &= ~O_TRUNC; > >> break; > > > > This will immediately break a system that runs code with MAY_OPENEXEC > > set but reads from a block, char, fifo, or socket, even in the case of > > a sysadmin leaving the "file" sysctl disabled. > > As documented, O_MAYEXEC is for regular files. The only legitimate use > case seems to be with pipes, which should probably be allowed when > enforcement is disabled. By the way Kees, while we fix that for the next series, do you think it would be relevant, at least for the sake of clarity, to add a WARN_ON_ONCE(acc_mode & MAY_OPENEXEC) for the S_IFSOCK case, since a socket cannot be open anyway? -- Thibaut Sautereau CLIP OS developer
