linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] ovl: copy up of disconnected dentries
@ 2021-03-22 12:18 Dan Carpenter
  2021-03-22 12:25 ` Amir Goldstein
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2021-03-22 12:18 UTC (permalink / raw)
  To: amir73il; +Cc: linux-unionfs

Hello Amir Goldstein,

The patch aa3ff3c152ff: "ovl: copy up of disconnected dentries" from
Oct 15, 2017, leads to the following static checker warning:

	fs/overlayfs/copy_up.c:972 ovl_copy_up_flags()
	warn: 'old_cred' not released on lines: 944.

fs/overlayfs/copy_up.c
   932  static int ovl_copy_up_flags(struct dentry *dentry, int flags)
   933  {
   934          int err = 0;
   935          const struct cred *old_cred = ovl_override_creds(dentry->d_sb);
   936          bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED);
   937  
   938          /*
   939           * With NFS export, copy up can get called for a disconnected non-dir.
   940           * In this case, we will copy up lower inode to index dir without
   941           * linking it to upper dir.
   942           */
   943          if (WARN_ON(disconnected && d_is_dir(dentry)))
   944                  return -EIO;

Should this call revert_creds(old_cred); before returning?

   945  
   946          while (!err) {
   947                  struct dentry *next;
   948                  struct dentry *parent = NULL;
   949  
   950                  if (ovl_already_copied_up(dentry, flags))
   951                          break;
   952  
   953                  next = dget(dentry);
   954                  /* find the topmost dentry not yet copied up */
   955                  for (; !disconnected;) {
   956                          parent = dget_parent(next);
   957  
   958                          if (ovl_dentry_upper(parent))
   959                                  break;
   960  
   961                          dput(next);
   962                          next = parent;
   963                  }
   964  
   965                  err = ovl_copy_up_one(parent, next, flags);
   966  
   967                  dput(parent);
   968                  dput(next);
   969          }
   970          revert_creds(old_cred);
   971  
   972          return err;
   973  }

regards,
dan carpenter

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

* Re: [bug report] ovl: copy up of disconnected dentries
  2021-03-22 12:18 [bug report] ovl: copy up of disconnected dentries Dan Carpenter
@ 2021-03-22 12:25 ` Amir Goldstein
  2021-03-22 12:35   ` Dan Carpenter
  0 siblings, 1 reply; 3+ messages in thread
From: Amir Goldstein @ 2021-03-22 12:25 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: overlayfs

On Mon, Mar 22, 2021 at 2:18 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Amir Goldstein,
>
> The patch aa3ff3c152ff: "ovl: copy up of disconnected dentries" from
> Oct 15, 2017, leads to the following static checker warning:

Heh! that's fashionably late :)

>
>         fs/overlayfs/copy_up.c:972 ovl_copy_up_flags()
>         warn: 'old_cred' not released on lines: 944.
>
> fs/overlayfs/copy_up.c
>    932  static int ovl_copy_up_flags(struct dentry *dentry, int flags)
>    933  {
>    934          int err = 0;
>    935          const struct cred *old_cred = ovl_override_creds(dentry->d_sb);
>    936          bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED);
>    937
>    938          /*
>    939           * With NFS export, copy up can get called for a disconnected non-dir.
>    940           * In this case, we will copy up lower inode to index dir without
>    941           * linking it to upper dir.
>    942           */
>    943          if (WARN_ON(disconnected && d_is_dir(dentry)))
>    944                  return -EIO;
>
> Should this call revert_creds(old_cred); before returning?

Yes. Here's a simple fix, care to post it?

Thanks,
Amir.

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 0b2891c6c71e..2846b943e80c 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -932,7 +932,7 @@ static int ovl_copy_up_one(struct dentry *parent,
struct dentry *dentry,
 static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 {
        int err = 0;
-       const struct cred *old_cred = ovl_override_creds(dentry->d_sb);
+       const struct cred *old_cred;
        bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED);

        /*
@@ -943,6 +943,7 @@ static int ovl_copy_up_flags(struct dentry
*dentry, int flags)
        if (WARN_ON(disconnected && d_is_dir(dentry)))
                return -EIO;

+       old_cred = ovl_override_creds(dentry->d_sb);
        while (!err) {
                struct dentry *next;
                struct dentry *parent = NULL;

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

* Re: [bug report] ovl: copy up of disconnected dentries
  2021-03-22 12:25 ` Amir Goldstein
@ 2021-03-22 12:35   ` Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2021-03-22 12:35 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: overlayfs

On Mon, Mar 22, 2021 at 02:25:44PM +0200, Amir Goldstein wrote:
> On Mon, Mar 22, 2021 at 2:18 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > Hello Amir Goldstein,
> >
> > The patch aa3ff3c152ff: "ovl: copy up of disconnected dentries" from
> > Oct 15, 2017, leads to the following static checker warning:
> 
> Heh! that's fashionably late :)
> 

;)

> >
> >         fs/overlayfs/copy_up.c:972 ovl_copy_up_flags()
> >         warn: 'old_cred' not released on lines: 944.
> >
> > fs/overlayfs/copy_up.c
> >    932  static int ovl_copy_up_flags(struct dentry *dentry, int flags)
> >    933  {
> >    934          int err = 0;
> >    935          const struct cred *old_cred = ovl_override_creds(dentry->d_sb);
> >    936          bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED);
> >    937
> >    938          /*
> >    939           * With NFS export, copy up can get called for a disconnected non-dir.
> >    940           * In this case, we will copy up lower inode to index dir without
> >    941           * linking it to upper dir.
> >    942           */
> >    943          if (WARN_ON(disconnected && d_is_dir(dentry)))
> >    944                  return -EIO;
> >
> > Should this call revert_creds(old_cred); before returning?
> 
> Yes. Here's a simple fix, care to post it?

Sure.  I'm always happy to take credit for your work.  I'll send it
tomorrow.

regards,
dan carpenter


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

end of thread, other threads:[~2021-03-22 12:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 12:18 [bug report] ovl: copy up of disconnected dentries Dan Carpenter
2021-03-22 12:25 ` Amir Goldstein
2021-03-22 12:35   ` Dan Carpenter

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).