From: Al Viro <[email protected]>
commit d0cb50185ae942b03c4327be322055d622dc79f6 upstream.
may_create_in_sticky() call is done when we already have dropped the
reference to dir.
Fixes: 30aba6656f61e (namei: allow restricted O_CREAT of FIFOs and regular files)
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
fs/namei.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1009,7 +1009,8 @@ static int may_linkat(struct path *link)
* may_create_in_sticky - Check whether an O_CREAT open in a sticky directory
* should be allowed, or not, on files that already
* exist.
- * @dir: the sticky parent directory
+ * @dir_mode: mode bits of directory
+ * @dir_uid: owner of directory
* @inode: the inode of the file to open
*
* Block an O_CREAT open of a FIFO (or a regular file) when:
@@ -1025,18 +1026,18 @@ static int may_linkat(struct path *link)
*
* Returns 0 if the open is allowed, -ve on error.
*/
-static int may_create_in_sticky(struct dentry * const dir,
+static int may_create_in_sticky(umode_t dir_mode, kuid_t dir_uid,
struct inode * const inode)
{
if ((!sysctl_protected_fifos && S_ISFIFO(inode->i_mode)) ||
(!sysctl_protected_regular && S_ISREG(inode->i_mode)) ||
- likely(!(dir->d_inode->i_mode & S_ISVTX)) ||
- uid_eq(inode->i_uid, dir->d_inode->i_uid) ||
+ likely(!(dir_mode & S_ISVTX)) ||
+ uid_eq(inode->i_uid, dir_uid) ||
uid_eq(current_fsuid(), inode->i_uid))
return 0;
- if (likely(dir->d_inode->i_mode & 0002) ||
- (dir->d_inode->i_mode & 0020 &&
+ if (likely(dir_mode & 0002) ||
+ (dir_mode & 0020 &&
((sysctl_protected_fifos >= 2 && S_ISFIFO(inode->i_mode)) ||
(sysctl_protected_regular >= 2 && S_ISREG(inode->i_mode))))) {
return -EACCES;
@@ -3258,6 +3259,8 @@ static int do_last(struct nameidata *nd,
struct file *file, const struct open_flags *op)
{
struct dentry *dir = nd->path.dentry;
+ kuid_t dir_uid = dir->d_inode->i_uid;
+ umode_t dir_mode = dir->d_inode->i_mode;
int open_flag = op->open_flag;
bool will_truncate = (open_flag & O_TRUNC) != 0;
bool got_write = false;
@@ -3393,7 +3396,7 @@ finish_open:
error = -EISDIR;
if (d_is_dir(nd->path.dentry))
goto out;
- error = may_create_in_sticky(dir,
+ error = may_create_in_sticky(dir_mode, dir_uid,
d_backing_inode(nd->path.dentry));
if (unlikely(error))
goto out;
On Tue, 2020-01-28 at 15:08 +0100, Greg Kroah-Hartman wrote:
> From: Al Viro <[email protected]>
>
> commit d0cb50185ae942b03c4327be322055d622dc79f6 upstream.
>
> may_create_in_sticky() call is done when we already have dropped the
> reference to dir.
>
> Fixes: 30aba6656f61e (namei: allow restricted O_CREAT of FIFOs and
> regular files)
> Signed-off-by: Al Viro <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> fs/namei.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> --- a/fs/namei.c
> +++ b/fs/namei.c
> [...]
> @@ -3258,6 +3259,8 @@ static int do_last(struct nameidata *nd,
> struct file *file, const struct open_flags *op)
> {
> struct dentry *dir = nd->path.dentry;
> + kuid_t dir_uid = dir->d_inode->i_uid;
I hit the following oops in 4.19.100 while running kselftests.
fs/namei.c:3262 matches the line above.
Any ideas?
-Tommi
[ 224.682489] test_firmware: batched loading 'tmp.YlpRSimRmc' custom
fallback mechanism 4 times
[ 224.686656] misc test_firmware: Direct firmware load for tmp.YlpRSimRmc
failed with error -2
[ 224.691091] misc test_firmware: Direct firmware load for tmp.YlpRSimRmc
failed with error -2
[ 228.593757] Scheduler tracepoints stat_sleep, stat_iowait, stat_blocked
and stat_runtime require the kernel parameter schedstats=enable or
kernel.sched_schedstats=1
[ 239.451836] BUG: unable to handle kernel NULL pointer dereference at
0000000000000004
[ 239.460454] PGD 0 P4D 0
[ 239.461875] Oops: 0000 [#1] SMP PTI
[ 239.463696] CPU: 0 PID: 13450 Comm: ftracetest Not tainted 4.19.100-
1.x86_64 #1
[ 239.467604] Hardware name: Red Hat OpenStack Compute, BIOS 1.11.0-2.el7
04/01/2014
[ 239.471254] RIP: 0010:path_openat (fs/namei.c:3262 fs/namei.c:3537)
[ 239.482135] RSP: 0018:ffffb301c10e7cf8 EFLAGS: 00010246
[ 239.484757] RAX: 0000000000000000 RBX: ffff99a8f57e1dc0 RCX:
0000000000000004
[ 239.488229] RDX: 0000000000000000 RSI: 0000000c00000000 RDI:
ffff99a881d49900
[ 239.507724] RBP: ffffb301c10e7de0 R08: ffffb301c10e7c44 R09:
61c8864680b583eb
[ 239.511222] R10: 8080807fffffffff R11: d0ffffff979c8b96 R12:
0000000000008241
[ 239.514709] R13: 0000000000000000 R14: ffff99a881d49900 R15:
ffffb301c10e7eec
[ 239.518287] FS: 00007f1bda771740(0000) GS:ffff99a8f7a00000(0000)
knlGS:0000000000000000
[ 239.522233] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 239.524964] CR2: 0000000000000004 CR3: 00000001c2196005 CR4:
00000000001606f0
[ 239.528309] DR0: 0000000000405118 DR1: 0000000000000000 DR2:
0000000000000000
[ 239.531599] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000600
[ 239.534946] Call Trace:
[ 239.536265] ? __switch_to_asm (arch/x86/entry/entry_64.S:374)
[ 239.538164] do_filp_open (fs/namei.c:3568)
[ 239.539885] ? __switch_to_asm (arch/x86/entry/entry_64.S:374)
[ 239.541747] ? __switch_to_asm (arch/x86/entry/entry_64.S:374)
[ 239.543633] do_sys_open (fs/open.c:1089)
[ 239.545392] do_syscall_64 (arch/x86/entry/common.c:293)
[ 239.547204] ? prepare_exit_to_usermode (arch/x86/entry/common.c:198)
[ 239.549424] entry_SYSCALL_64_after_hwframe
(arch/x86/entry/entry_64.S:247)
[ 239.551815] RIP: 0033:0x7f1bda86617b
[ 239.561833] RSP: 002b:00007fffbd62ff10 EFLAGS: 00000246 ORIG_RAX:
0000000000000101
[ 239.565335] RAX: ffffffffffffffda RBX: 0000559cde8e9480 RCX:
00007f1bda86617b
[ 239.568576] RDX: 0000000000000241 RSI: 0000559cde908d90 RDI:
00000000ffffff9c
[ 239.571810] RBP: 0000559cde908d90 R08: 0000000000000000 R09:
0000000000000020
[ 239.575054] R10: 00000000000001b6 R11: 0000000000000246 R12:
0000000000000241
[ 239.578277] R13: 0000000000000000 R14: 0000000000000001 R15:
0000559cde908d90
[ 239.581512] Modules linked in: test_firmware kvm_intel kvm irqbypass
sch_fq_codel pcbc aesni_intel aes_x86_64 crypto_simd cryptd glue_helper
ata_piix dm_mirror dm_region_hash dm_log dm_mod autofs4
[ 239.589275] CR2: 0000000000000004
[ 239.590972] ---[ end trace 4b84bf2ccd45a421 ]---
[ 239.593205] RIP: 0010:path_openat (fs/namei.c:3262 fs/namei.c:3537)
[ 239.603919] RSP: 0018:ffffb301c10e7cf8 EFLAGS: 00010246
[ 239.606473] RAX: 0000000000000000 RBX: ffff99a8f57e1dc0 RCX:
0000000000000004
[ 239.609775] RDX: 0000000000000000 RSI: 0000000c00000000 RDI:
ffff99a881d49900
[ 239.613014] RBP: ffffb301c10e7de0 R08: ffffb301c10e7c44 R09:
61c8864680b583eb
[ 239.616244] R10: 8080807fffffffff R11: d0ffffff979c8b96 R12:
0000000000008241
[ 239.619457] R13: 0000000000000000 R14: ffff99a881d49900 R15:
ffffb301c10e7eec
[ 239.622911] FS: 00007f1bda771740(0000) GS:ffff99a8f7a00000(0000)
knlGS:0000000000000000
[ 239.626611] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 239.629254] CR2: 0000000000000004 CR3: 00000001c2196005 CR4:
00000000001606f0
[ 239.632512] DR0: 0000000000405118 DR1: 0000000000000000 DR2:
0000000000000000
[ 239.635745] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000600
[ 243.805958] trace_kprobe: Parse error at argument[0]. (-22)
[ 243.879983] trace_probe: x64 type has no corresponding fetch method.
[ 243.882922] trace_kprobe: Parse error at argument[0]. (-22)
> + umode_t dir_mode = dir->d_inode->i_mode;
> int open_flag = op->open_flag;
> bool will_truncate = (open_flag & O_TRUNC) != 0;
> bool got_write = false;
> @@ -3393,7 +3396,7 @@ finish_open:
> error = -EISDIR;
> if (d_is_dir(nd->path.dentry))
> goto out;
> - error = may_create_in_sticky(dir,
> + error = may_create_in_sticky(dir_mode, dir_uid,
> d_backing_inode(nd-
> >path.dentry));
> if (unlikely(error))
> goto out;
>
>
On Fri, Jan 31, 2020 at 10:08:37AM +0000, Rantala, Tommi T. (Nokia - FI/Espoo) wrote:
> On Tue, 2020-01-28 at 15:08 +0100, Greg Kroah-Hartman wrote:
> > From: Al Viro <[email protected]>
> >
> > commit d0cb50185ae942b03c4327be322055d622dc79f6 upstream.
> >
> > may_create_in_sticky() call is done when we already have dropped the
> > reference to dir.
> >
> > Fixes: 30aba6656f61e (namei: allow restricted O_CREAT of FIFOs and
> > regular files)
> > Signed-off-by: Al Viro <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
> > ---
> > fs/namei.c | 17 ++++++++++-------
> > 1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > [...]
> > @@ -3258,6 +3259,8 @@ static int do_last(struct nameidata *nd,
> > struct file *file, const struct open_flags *op)
> > {
> > struct dentry *dir = nd->path.dentry;
> > + kuid_t dir_uid = dir->d_inode->i_uid;
>
> I hit the following oops in 4.19.100 while running kselftests.
>
> fs/namei.c:3262 matches the line above.
>
> Any ideas?
Yes. Make those two line
kuid_t dir_uid = nd->inode->i_uid;
umode_t dir_mode = nd->inode->i_mode;
I'm pretty sure that I know which way I'd fucked up there; we can
get here in RCU mode with stale nd->path.dentry (that would make
the thing fail with -ECHILD. with retry in non-RCU mode). In
non-stale case nd->inode is the same as nd->path.dentry->d_inode
and it's always pointing to a struct inode that hadn't been
freed yet.
On Fri, 2020-01-31 at 12:20 +0000, Al Viro wrote:
> On Fri, Jan 31, 2020 at 10:08:37AM +0000, Rantala, Tommi T. (Nokia -
> FI/Espoo) wrote:
> > On Tue, 2020-01-28 at 15:08 +0100, Greg Kroah-Hartman wrote:
> > > From: Al Viro <[email protected]>
> > >
> > > @@ -3258,6 +3259,8 @@ static int do_last(struct nameidata *nd,
> > > struct file *file, const struct open_flags *op)
> > > {
> > > struct dentry *dir = nd->path.dentry;
> > > + kuid_t dir_uid = dir->d_inode->i_uid;
> >
> > I hit the following oops in 4.19.100 while running kselftests.
> >
> > fs/namei.c:3262 matches the line above.
> >
> > Any ideas?
>
> Yes. Make those two line
> kuid_t dir_uid = nd->inode->i_uid;
> umode_t dir_mode = nd->inode->i_mode;
>
> I'm pretty sure that I know which way I'd fucked up there; we can
> get here in RCU mode with stale nd->path.dentry (that would make
> the thing fail with -ECHILD. with retry in non-RCU mode). In
> non-stale case nd->inode is the same as nd->path.dentry->d_inode
> and it's always pointing to a struct inode that hadn't been
> freed yet.
The oops does not seem to reproduce easily with kselftests, I've only hit
it once with 4.19.100.
Made the change in fs/namei.c, so far no ill effects.
I'll continue running the kselftests...
-Tommi