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

[Muhammad Usama Anjum <Usama.Anjum@collabora.com>]

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