mm: remove the newlines, which are added for unknown reasons and interfere with bug analysis

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

Posted by Jeongjun Park 1 month, 3 weeks ago

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@vger.kernel.org
Signed-off-by: Jeongjun Park <aha310510@gmail.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 @@
-
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  *  linux/mm/memory.c
--

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

Posted by Greg KH 1 month, 3 weeks ago

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@vger.kernel.org
> Signed-off-by: Jeongjun Park <aha310510@gmail.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?

Also, your changelog text has trailing whitespace, ironic for a patch
that does a whitespace cleanup :)

thanks,

greg k-h

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

Posted by Jeongjun Park 1 month, 3 weeks ago

Greg KH <gregkh@linuxfoundation.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@vger.kernel.org
> > Signed-off-by: Jeongjun Park <aha310510@gmail.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.

>
> Also, your changelog text has trailing whitespace, ironic for a patch
> that does a whitespace cleanup :)
>

Oops, I forgot to remove the trailing space in the patch description.
If you absolutely must remove the space, I'll write a v2 patch and
send it to you.

Regards,
Jeongjun Park

> thanks,
>
> greg k-h

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

Posted by Greg KH 1 month, 3 weeks ago

On Mon, Oct 07, 2024 at 05:57:18PM +0900, Jeongjun Park wrote:
> Greg KH <gregkh@linuxfoundation.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@vger.kernel.org
> > > Signed-off-by: Jeongjun Park <aha310510@gmail.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

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

Posted by Jeongjun Park 1 month, 3 weeks ago

Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Oct 07, 2024 at 05:57:18PM +0900, Jeongjun Park wrote:
> > Greg KH <gregkh@linuxfoundation.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@vger.kernel.org
> > > > Signed-off-by: Jeongjun Park <aha310510@gmail.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.
>

Oh, I just realized that this website is open source and written in C.

This seems to be the correct git repository, so I'll commit here.

https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/cgit.git

Regards,
Jeongjun Park

> thanks,
>
> greg k-h

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

Posted by Andrew Morton 1 month, 2 weeks ago

On Mon, 7 Oct 2024 20:24:57 +0900 Jeongjun Park <aha310510@gmail.com> wrote:

> > > 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.
> >
> 
> Oh, I just realized that this website is open source and written in C.
> 
> This seems to be the correct git repository, so I'll commit here.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/cgit.git

Still, we don't want that newline in there.  I did this as a little
cleanup.  Please send a new (similar) patch if you're prefer.


From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm/memory.c: remove stray newline at top of file
Date: Mon Oct  7 06:20:09 PM PDT 2024

Fixes: d61ea1cb0095 ("userfaultfd: UFFD_FEATURE_WP_ASYNC")
Reported-by: Jeongjun Park <aha310510@gmail.com>
Closes: https://lkml.kernel.org/r/20241007065307.4158-1-aha310510@gmail.com
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/memory.c |    1 -
 1 file changed, 1 deletion(-)

--- a/mm/memory.c~mm-memoryc-remove-stray-newline-at-top-of-file
+++ a/mm/memory.c
@@ -1,4 +1,3 @@
-
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  *  linux/mm/memory.c
_

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

Posted by Muhammad Usama Anjum 1 month, 3 weeks ago

On 10/7/24 4:24 PM, Jeongjun Park wrote:
> Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Mon, Oct 07, 2024 at 05:57:18PM +0900, Jeongjun Park wrote:
>>> Greg KH <gregkh@linuxfoundation.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@vger.kernel.org
>>>>> Signed-off-by: Jeongjun Park <aha310510@gmail.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.
>>
> 
> Oh, I just realized that this website is open source and written in C.
> 
> This seems to be the correct git repository, so I'll commit here.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/cgit.git
Get latest tag from
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/
instead.

https://kernelnewbies.org/FirstKernelPatch could be helpful in
understanding some missing details.

> 
> Regards,
> Jeongjun Park
> 
>> thanks,
>>
>> greg k-h

-- 
BR,
Muhammad Usama Anjum