linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced
@ 2022-11-03 17:02 Peter Griffin
  2022-11-03 19:11 ` Miklos Szeredi
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Griffin @ 2022-11-03 17:02 UTC (permalink / raw)
  Cc: Peter Griffin, Alexander Viro, Miklos Szeredi, stable,
	linux-fsdevel, linux-kernel, Will McVicker, Peter Griffin

If O_EXCL is *not* specified, then linkat() can be
used to link the temporary file into the filesystem.
If O_EXCL is specified then linkat() should fail (-1).

After commit 863f144f12ad ("vfs: open inside ->tmpfile()")
the O_EXCL flag is no longer honored by the vfs layer for
tmpfile, which means the file can be linked even if O_EXCL
flag is specified, which is a change in behaviour for
userspace!

The open flags was previously passed as a parameter, so it
was uneffected by the changes to file->f_flags caused by
finish_open(). This patch fixes the issue by storing
file->f_flags in a local variable so the O_EXCL test
logic is restored.

This regression was detected by Android CTS Bionic fcntl()
tests running on android-mainline [1].

[1] https://android.googlesource.com/platform/bionic/+/
    refs/heads/master/tests/fcntl_test.cpp#352

Fixes: 863f144f12ad ("vfs: open inside ->tmpfile()")
To: lkml <linux-kernel@vger.kernel.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Miklos Szeredi <mszeredi@redhat.com>
Cc: stable@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Will McVicker <willmcvicker@google.com>
Cc: Peter Griffin <gpeter@google.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 fs/namei.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 578c2110df02..9155ecb547ce 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3591,6 +3591,7 @@ static int vfs_tmpfile(struct user_namespace *mnt_userns,
 	struct inode *dir = d_inode(parentpath->dentry);
 	struct inode *inode;
 	int error;
+	int open_flag = file->f_flags;
 
 	/* we want directory to be writable */
 	error = inode_permission(mnt_userns, dir, MAY_WRITE | MAY_EXEC);
@@ -3613,7 +3614,7 @@ static int vfs_tmpfile(struct user_namespace *mnt_userns,
 	if (error)
 		return error;
 	inode = file_inode(file);
-	if (!(file->f_flags & O_EXCL)) {
+	if (!(open_flag & O_EXCL)) {
 		spin_lock(&inode->i_lock);
 		inode->i_state |= I_LINKABLE;
 		spin_unlock(&inode->i_lock);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced
  2022-11-03 17:02 [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced Peter Griffin
@ 2022-11-03 19:11 ` Miklos Szeredi
  2022-11-03 19:33   ` William McVicker
  2022-11-14 14:38   ` Peter Griffin
  0 siblings, 2 replies; 5+ messages in thread
From: Miklos Szeredi @ 2022-11-03 19:11 UTC (permalink / raw)
  To: Peter Griffin
  Cc: Alexander Viro, Miklos Szeredi, stable, linux-fsdevel,
	linux-kernel, Will McVicker, Peter Griffin

On Thu, 3 Nov 2022 at 18:04, Peter Griffin <peter.griffin@linaro.org> wrote:
>
> If O_EXCL is *not* specified, then linkat() can be
> used to link the temporary file into the filesystem.
> If O_EXCL is specified then linkat() should fail (-1).
>
> After commit 863f144f12ad ("vfs: open inside ->tmpfile()")
> the O_EXCL flag is no longer honored by the vfs layer for
> tmpfile, which means the file can be linked even if O_EXCL
> flag is specified, which is a change in behaviour for
> userspace!
>
> The open flags was previously passed as a parameter, so it
> was uneffected by the changes to file->f_flags caused by
> finish_open(). This patch fixes the issue by storing
> file->f_flags in a local variable so the O_EXCL test
> logic is restored.
>
> This regression was detected by Android CTS Bionic fcntl()
> tests running on android-mainline [1].
>
> [1] https://android.googlesource.com/platform/bionic/+/
>     refs/heads/master/tests/fcntl_test.cpp#352

Looks good.

Acked-by: Miklos Szeredi <mszeredi@redhat.com>

Thanks,
Miklos
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced
  2022-11-03 19:11 ` Miklos Szeredi
@ 2022-11-03 19:33   ` William McVicker
  2022-11-14 14:38   ` Peter Griffin
  1 sibling, 0 replies; 5+ messages in thread
From: William McVicker @ 2022-11-03 19:33 UTC (permalink / raw)
  To: Peter Griffin
  Cc: Peter Griffin, Alexander Viro, Miklos Szeredi, stable,
	linux-fsdevel, linux-kernel, Peter Griffin

On 11/03/2022, Miklos Szeredi wrote:
> On Thu, 3 Nov 2022 at 18:04, Peter Griffin <peter.griffin@linaro.org> wrote:
> >
> > If O_EXCL is *not* specified, then linkat() can be
> > used to link the temporary file into the filesystem.
> > If O_EXCL is specified then linkat() should fail (-1).
> >
> > After commit 863f144f12ad ("vfs: open inside ->tmpfile()")
> > the O_EXCL flag is no longer honored by the vfs layer for
> > tmpfile, which means the file can be linked even if O_EXCL
> > flag is specified, which is a change in behaviour for
> > userspace!
> >
> > The open flags was previously passed as a parameter, so it
> > was uneffected by the changes to file->f_flags caused by
> > finish_open(). This patch fixes the issue by storing
> > file->f_flags in a local variable so the O_EXCL test
> > logic is restored.
> >
> > This regression was detected by Android CTS Bionic fcntl()
> > tests running on android-mainline [1].
> >
> > [1] https://android.googlesource.com/platform/bionic/+/
> >     refs/heads/master/tests/fcntl_test.cpp#352
> 
> Looks good.
> 
> Acked-by: Miklos Szeredi <mszeredi@redhat.com>
> 
> Thanks,
> Miklos
> >

Thanks Peter for tracking this down! I tested this on the android-mainline
version of 6.1-rc3 on a Pixel 6 device.

Tested-by: Will McVicker <willmcvicker@google.com>

Regards,
Will

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced
  2022-11-03 19:11 ` Miklos Szeredi
  2022-11-03 19:33   ` William McVicker
@ 2022-11-14 14:38   ` Peter Griffin
  2022-11-19  7:25     ` Al Viro
  1 sibling, 1 reply; 5+ messages in thread
From: Peter Griffin @ 2022-11-14 14:38 UTC (permalink / raw)
  To: Alexander Viro
  Cc: Miklos Szeredi, stable, linux-fsdevel, linux-kernel,
	Will McVicker, Peter Griffin

Hi Alexander,

On Thu, 3 Nov 2022 at 19:12, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Thu, 3 Nov 2022 at 18:04, Peter Griffin <peter.griffin@linaro.org> wrote:
> >
> > If O_EXCL is *not* specified, then linkat() can be
> > used to link the temporary file into the filesystem.
> > If O_EXCL is specified then linkat() should fail (-1).
> >
> > After commit 863f144f12ad ("vfs: open inside ->tmpfile()")
> > the O_EXCL flag is no longer honored by the vfs layer for
> > tmpfile, which means the file can be linked even if O_EXCL
> > flag is specified, which is a change in behaviour for
> > userspace!
> >
> > The open flags was previously passed as a parameter, so it
> > was uneffected by the changes to file->f_flags caused by
> > finish_open(). This patch fixes the issue by storing
> > file->f_flags in a local variable so the O_EXCL test
> > logic is restored.
> >
> > This regression was detected by Android CTS Bionic fcntl()
> > tests running on android-mainline [1].
> >
> > [1] https://android.googlesource.com/platform/bionic/+/
> >     refs/heads/master/tests/fcntl_test.cpp#352
>
> Looks good.
>
> Acked-by: Miklos Szeredi <mszeredi@redhat.com>

As this patch now has an Acked-by the original author of the
commit that reworked the tmpfile vfs logic and introduced the
regression. Can you pick up this commit and send it onto Linus
for inclusion into the next v6.1-rc release?

Note, it fixes a regression for userspace introduced in this merge
window so I was hoping to get the fix into the next -rc so that the
v6.1 release does not contain this bug.

Many thanks,

Peter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced
  2022-11-14 14:38   ` Peter Griffin
@ 2022-11-19  7:25     ` Al Viro
  0 siblings, 0 replies; 5+ messages in thread
From: Al Viro @ 2022-11-19  7:25 UTC (permalink / raw)
  To: Peter Griffin
  Cc: Miklos Szeredi, stable, linux-fsdevel, linux-kernel,
	Will McVicker, Peter Griffin

On Mon, Nov 14, 2022 at 02:38:15PM +0000, Peter Griffin wrote:

> commit that reworked the tmpfile vfs logic and introduced the
> regression. Can you pick up this commit and send it onto Linus
> for inclusion into the next v6.1-rc release?
> 
> Note, it fixes a regression for userspace introduced in this merge
> window so I was hoping to get the fix into the next -rc so that the
> v6.1 release does not contain this bug.

Applied to #fixes and pushed out; will send a pull request to Linus
tomorrow...

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-11-19  7:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 17:02 [PATCH] vfs: vfs_tmpfile: ensure O_EXCL flag is enforced Peter Griffin
2022-11-03 19:11 ` Miklos Szeredi
2022-11-03 19:33   ` William McVicker
2022-11-14 14:38   ` Peter Griffin
2022-11-19  7:25     ` Al Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).