linux-kernel - Re: [PATCH] mm: remove the newlines, which are added for unknown reasons and interfere with bug analysis

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: <2024100700-animal-upriver-fb7c@gregkh>
Date: Mon, 7 Oct 2024 11:05:38 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Jeongjun Park <aha310510@...il.com>
Cc: akpm@...ux-foundation.org, usama.anjum@...labora.com, peterx@...hat.com,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] mm: remove the newlines, which are added for unknown
 reasons and interfere with bug analysis

On Mon, Oct 07, 2024 at 05:57:18PM +0900, Jeongjun Park wrote:
> Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Mon, Oct 07, 2024 at 03:53:07PM +0900, Jeongjun Park wrote:
> > > Looking at the source code links for mm/memory.c in the sample reports
> > > in the syzbot report links [1].
> > >
> > > it looks like the line numbers are designated as lines that have been
> > > increased by 1. This may seem like a problem with syzkaller or the
> > > addr2line program that assigns the line numbers, but there is no problem
> > > with either of them.
> > >
> > > In the previous commit d61ea1cb0095 ("userfaultfd: UFFD_FEATURE_WP_ASYNC"),
> > > when modifying mm/memory.c, an unknown line break is added to the very first
> > > line of the file. However, the git.kernel.org site displays the source code
> > > with the added line break removed, so even though addr2line has assigned
> > > the correct line number, it looks like the line number has increased by 1.
> > >
> > > This may seem like a trivial thing, but I think it would be appropriate
> > > to remove all the newline characters added to the upstream and stable
> > > versions, as they are not only incorrect in terms of code style but also
> > > hinder bug analysis.
> > >
> > > [1]
> > >
> > > https://syzkaller.appspot.com/bug?extid=4145b11cdf925264bff4
> > > https://syzkaller.appspot.com/bug?extid=fa43f1b63e3aa6f66329
> > > https://syzkaller.appspot.com/bug?extid=890a1df7294175947697
> > >
> > > Fixes: d61ea1cb0095 ("userfaultfd: UFFD_FEATURE_WP_ASYNC")
> > > Cc: stable@...r.kernel.org
> > > Signed-off-by: Jeongjun Park <aha310510@...il.com>
> > > ---
> > >  mm/memory.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 2366578015ad..7dffe8749014 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -1,4 +1,3 @@
> > > -
> >
> > This sounds like you have broken tools that can not handle an empty line
> > in a file.
> >
> > Why not fix those?
> 
> As I mentioned above, there is no problem with addr2line's ability to parse
> the code line that called the function in the calltrace of the crash report.
> 
> However, when the source code of mm/memory.c is printed on the screen on the
> git.kernel.org site, the line break character that exists in the first line
> of the file is deleted and printed, so as a result, all code lines in the
> mm/memory.c file are located at line numbers that are -1 less than the
> actual line.
> 
> You can understand it easily if you compare the source code of mm/memory.c
> on github and git.kernel.org.
> 
> https://github.com/torvalds/linux/blob/master/mm/memory.c
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/memory.c
> 
> Since I cannot modify the source code printing function of the git.kernel.org
> site, the best solution I can suggest is to remove the unnecessary line break
> character that exists in all versions.

I would recommend fixing the git.kernel.org code, it is all open source
and can be fixed up, as odds are other projects/repos would like to have
it fixed as well.

thanks,

greg k-h