linux-kernel - Re: [PATCH] ocfs2: update seq_file index in ocfs2_dlm_seq

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: <20250214155515.6283b71e47730d0e2e6c9664@linux-foundation.org>
Date: Fri, 14 Feb 2025 15:55:15 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Joseph Qi <joseph.qi@...ux.alibaba.com>
Cc: Wengang Wang <wen.gang.wang@...cle.com>, "ocfs2-devel@...ts.linux.dev"
 <ocfs2-devel@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ocfs2: update seq_file index in ocfs2_dlm_seq_next

On Mon, 11 Nov 2024 17:35:49 +0800 Joseph Qi <joseph.qi@...ux.alibaba.com> wrote:

> 
> 
> On 11/11/24 3:04 PM, Wengang Wang wrote:
> > 
> > 
> >> On Nov 10, 2024, at 5:38 PM, Joseph Qi <joseph.qi@...ux.alibaba.com> wrote:
> >>
> >>
> >>
> >> On 11/9/24 3:28 AM, Wengang Wang wrote:
> >>> The following INFO level message was seen:
> >>>
> >>> seq_file: buggy .next function ocfs2_dlm_seq_next [ocfs2] did not
> >>> update position index
> >>>
> >>> Fix:
> >>> Updata m->index to make seq_read_iter happy though the index its self makes
> >>> no sense to ocfs2_dlm_seq_next.
> >>>
> >>> Signed-off-by: Wengang Wang <wen.gang.wang@...cle.com>
> >>> ---
> >>> fs/ocfs2/dlmglue.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> >>> index 60df52e4c1f8..349d131369cf 100644
> >>> --- a/fs/ocfs2/dlmglue.c
> >>> +++ b/fs/ocfs2/dlmglue.c
> >>> @@ -3120,6 +3120,7 @@ static void *ocfs2_dlm_seq_next(struct seq_file *m, void *v, loff_t *pos)
> >>> }
> >>> spin_unlock(&ocfs2_dlm_tracking_lock);
> >>>
> >>> + m->index++;
> >>
> >> We can directly use '(*pos)++' instead.
> >>
> > 
> > The input/output "pos” indicates more an offset into the file. Actually the output for an item is not really 1 byte in length, so incrementing the offset by 1 sounds a bit strange to me. Instead If we increment the “index”, It would be easier to understand it as  for next item. Though updating “index” or updating “*pos” instead makes no difference to binary running, the code understanding is different.  I know other seq_operations.next functions are directly incrementing the “*pos”, I think updating “index” is better. Well, if you persist (*pos)++, I will also let it go.
> > 
> >From seq_read_iter(), the input pos is equivalent to '&m->index'. So the
> above two ways seems have no functional difference.
> IMO, we'd better hide the m->index logic into seqfile and just use pos
> instead like other .next implementations.

Did we ever fix this?