linux-kernel - Re: [PATCH] NFS: Correct timing for assigning access cache timestamp

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAPza5qe=e-CkG+j4NYzF24cv5ZDFbheOFfAi+LQ_OW0pZmfc9g@mail.gmail.com>
Date:   Fri, 10 Mar 2023 10:57:01 +0800
From:   Chengen Du <chengen.du@...onical.com>
To:     trond.myklebust@...merspace.com
Cc:     Benjamin Coddington <bcodding@...hat.com>, anna@...nel.org,
        linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] NFS: Correct timing for assigning access cache timestamp

Hi,

We would like to inform you that we have utilized the patch to create
a test kernel,
which has been made available for community testing.
We are pleased to report that the feedback received thus far indicates
that the patch has successfully resolved the issue at hand.

Please refer to the following link for further details on the related
bug report:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2009325

Thank you for your attention to this matter.

Best regards,
Chengen Du

On Wed, Mar 8, 2023 at 10:45 PM Benjamin Coddington <bcodding@...hat.com> wrote:
>
> On 8 Mar 2023, at 3:03, Chengen Du wrote:
>
> > When the user's login time is newer than the cache's timestamp,
> > the original entry in the RB-tree will be replaced by a new entry.
> > Currently, the timestamp is only set if the entry is not found in
> > the RB-tree, which can cause the timestamp to be undefined when
> > the entry exists. This may result in a significant increase in
> > ACCESS operations if the timestamp is set to zero.
> >
> > Signed-off-by: Chengen Du <chengen.du@...onical.com>
> > ---
> >  fs/nfs/dir.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index a41c3ee4549c..6fbcbb8d6587 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -3089,7 +3089,6 @@ static void nfs_access_add_rbtree(struct inode *inode,
> >               else
> >                       goto found;
> >       }
> > -     set->timestamp = ktime_get_ns();
> >       rb_link_node(&set->rb_node, parent, p);
> >       rb_insert_color(&set->rb_node, root_node);
> >       list_add_tail(&set->lru, &nfsi->access_cache_entry_lru);
> > @@ -3114,6 +3113,7 @@ void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set,
> >       cache->fsgid = cred->fsgid;
> >       cache->group_info = get_group_info(cred->group_info);
> >       cache->mask = set->mask;
> > +     cache->timestamp = ktime_get_ns();
> >
> >       /* The above field assignments must be visible
> >        * before this item appears on the lru.  We cannot easily
> > --
> > 2.37.2
>
> Looks good to me.
>
> Reviewed-by: Benjamin Coddington <bcodding@...hat.com>
>