linux-kernel - Re: [PATCH 4.19 114/131] ocfs2: avoid inode removal while nfsd is accessing it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Jul 2020 15:24:29 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Pavel Machek <pavel@...x.de>
Cc:     Sasha Levin <sashal@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>,
        Junxiao Bi <junxiao.bi@...cle.com>,
        Joseph Qi <joseph.qi@...ux.alibaba.com>,
        Changwei Ge <gechangwei@...e.cn>, Gang He <ghe@...e.com>,
        Joel Becker <jlbec@...lplan.org>,
        Jun Piao <piaojun@...wei.com>, Mark Fasheh <mark@...heh.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 4.19 114/131] ocfs2: avoid inode removal while nfsd is
 accessing it

On Thu, Jul 2, 2020 at 2:17 PM Pavel Machek <pavel@...x.de> wrote:
>
>
> > commit 4cd9973f9ff69e37dd0ba2bd6e6423f8179c329a upstream.
> >
> > Patch series "ocfs2: fix nfsd over ocfs2 issues", v2.
>
> This causes locking imbalance:

This sems to be true upstream too.

> When ocfs2_nfs_sync_lock() returns error, caller can not know if the
> lock was taken or not.

Right you are.

And your patch looks sane:

> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index c141b06811a6..8149fb6f1f0d 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -2867,9 +2867,15 @@ int ocfs2_nfs_sync_lock(struct ocfs2_super *osb, int ex)
>
>         status = ocfs2_cluster_lock(osb, lockres, ex ? LKM_EXMODE : LKM_PRMODE,
>                                     0, 0);
> -       if (status < 0)
> +       if (status < 0) {
>                 mlog(ML_ERROR, "lock on nfs sync lock failed %d\n", status);
>
> +               if (ex)
> +                       up_write(&osb->nfs_sync_rwlock);
> +               else
> +                       up_read(&osb->nfs_sync_rwlock);
> +       }
> +
>         return status;
>  }

although the whole thing looks messy.

If the issue is a lifetime thing (like that commit says), the proper
model isn't a lock, but a refcount.

Oh well. Junxiao?

               Linus