[PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Loic]

Hello,

Please picked up this patch for linux 4.4 and 4.9.
This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)

Thank.

[ Upstream commit e09e28671cda63e6308b31798b997639120e2a21 ]

From: Amey Telawane <ameyt@codeaurora.org>
Date: Wed, 3 May 2017 15:41:14 +0530
Subject: [PATCH] tracing: Use strlcpy() instead of strcpy() in
 __trace_find_cmdline()

Strcpy is inherently not safe, and strlcpy() should be used instead.
__trace_find_cmdline() uses strcpy() because the comms saved must have a
terminating nul character, but it doesn't hurt to add the extra protection
of using strlcpy() instead of strcpy().

Link: http://lkml.kernel.org/r/1493806274-13936-1-git-send-email-amit.pundir@linaro.org

Signed-off-by: Amey Telawane <ameyt@codeaurora.org>
[AmitP: Cherry-picked this commit from CodeAurora kernel/msm-3.10
https://source.codeaurora.org/quic/la/kernel/msm-3.10/commit/?id=2161ae9a70b12cf18ac8e5952a20161ffbccb477]
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
[ Updated change log and removed the "- 1" from len parameter ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1862,7 +1862,7 @@ static void __trace_find_cmdline(int pid
 
 	map = savedcmd->map_pid_to_cmdline[pid];
 	if (map != NO_CMDLINE_MAP)
-		strcpy(comm, get_saved_cmdlines(map));
+		strlcpy(comm, get_saved_cmdlines(map), TASK_COMM_LEN);
 	else
 		strcpy(comm, "<...>");
 }

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Salvatore Bonaccorso]

Hi,

On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
> Hello,
> 
> Please picked up this patch for linux 4.4 and 4.9.
> This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)

https://marc.info/?l=oss-security&m=150703005326252&w=2 gives some
background on why the fix was not backported for 4.9.

Regards,
Salvatore

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Greg KH]

On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
> Hello,
> 
> Please picked up this patch for linux 4.4 and 4.9.
> This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)

It was rejected as a CVE for a good reason, and that reason is also why
I refused to add it to the stable kernel releases.  In short, this is
not an issue or bug at all, there is nothing wrong with the existing
code.

thanks,

greg k-h

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Steven Rostedt]

On Sun, 16 Dec 2018 09:52:33 +0100
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
> > Hello,
> > 
> > Please picked up this patch for linux 4.4 and 4.9.
> > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)  
> 
> It was rejected as a CVE for a good reason, and that reason is also why
> I refused to add it to the stable kernel releases.  In short, this is
> not an issue or bug at all, there is nothing wrong with the existing
> code.
>

I'm starting to regret that I ever accepted the original patch :-(

-- Steve

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Loic]

Le 2018-12-16 20:27, Steven Rostedt a écrit :
> On Sun, 16 Dec 2018 09:52:33 +0100
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
>> On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
>> > Hello,
>> >
>> > Please picked up this patch for linux 4.4 and 4.9.
>> > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
>> 
>> It was rejected as a CVE for a good reason, and that reason is also 
>> why
>> I refused to add it to the stable kernel releases.  In short, this is
>> not an issue or bug at all, there is nothing wrong with the existing
>> code.
>> 
> 
> I'm starting to regret that I ever accepted the original patch :-(
> 
> -- Steve

Okay, I hadn't looked at the previous conversations because this change 
is in the upstream and in debian...

-- 
Best regards,

Loic

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Greg KH]

On Sun, Dec 16, 2018 at 09:08:20PM +0100, Loic wrote:
> Le 2018-12-16 20:27, Steven Rostedt a �crit�:
> > On Sun, 16 Dec 2018 09:52:33 +0100
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
> > > > Hello,
> > > >
> > > > Please picked up this patch for linux 4.4 and 4.9.
> > > > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
> > > 
> > > It was rejected as a CVE for a good reason, and that reason is also
> > > why
> > > I refused to add it to the stable kernel releases.  In short, this is
> > > not an issue or bug at all, there is nothing wrong with the existing
> > > code.
> > > 
> > 
> > I'm starting to regret that I ever accepted the original patch :-(
> > 
> > -- Steve
> 
> Okay, I hadn't looked at the previous conversations because this change is
> in the upstream and in debian...

Upstream is fine, it's a valid change so that people don't keep sending
the crazy patch over and over.

Debian is just cargo-culting the thing and should probably drop it as it
keeps coming back to me every 3 months or so, and I have to reject it
again :(

thanks,

greg k-h

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Loic]

Le 2018-12-17 09:19, Greg KH a écrit :
> On Sun, Dec 16, 2018 at 09:08:20PM +0100, Loic wrote:
>> Le 2018-12-16 20:27, Steven Rostedt a écrit :
>> > On Sun, 16 Dec 2018 09:52:33 +0100
>> > Greg KH <gregkh@linuxfoundation.org> wrote:
>> >
>> > > On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
>> > > > Hello,
>> > > >
>> > > > Please picked up this patch for linux 4.4 and 4.9.
>> > > > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
>> > >
>> > > It was rejected as a CVE for a good reason, and that reason is also
>> > > why
>> > > I refused to add it to the stable kernel releases.  In short, this is
>> > > not an issue or bug at all, there is nothing wrong with the existing
>> > > code.
>> > >
>> >
>> > I'm starting to regret that I ever accepted the original patch :-(
>> >
>> > -- Steve
>> 
>> Okay, I hadn't looked at the previous conversations because this 
>> change is
>> in the upstream and in debian...
> 
> Upstream is fine, it's a valid change so that people don't keep sending
> the crazy patch over and over.
> 
> Debian is just cargo-culting the thing and should probably drop it as 
> it
> keeps coming back to me every 3 months or so, and I have to reject it
> again :(
> 
> thanks,
> 
> greg k-h

Why didn't you follow the upstream or add a comment "no change for fake 
CVE-2017-0605" to break the debian patch ?

In short, I accuse the Debian kernel team in my defense, it's up to them 
to buy you a beer :)

Thanks.

Best regards,

Loic

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Sasha Levin]

On Mon, Dec 17, 2018 at 08:42:38PM +0100, Loic wrote:
>Le 2018-12-17 09:19, Greg KH a �crit�:
>>On Sun, Dec 16, 2018 at 09:08:20PM +0100, Loic wrote:
>>>Le 2018-12-16 20:27, Steven Rostedt a �crit�:
>>>> On Sun, 16 Dec 2018 09:52:33 +0100
>>>> Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>
>>>> > On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
>>>> > > Hello,
>>>> > >
>>>> > > Please picked up this patch for linux 4.4 and 4.9.
>>>> > > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
>>>> >
>>>> > It was rejected as a CVE for a good reason, and that reason is also
>>>> > why
>>>> > I refused to add it to the stable kernel releases.  In short, this is
>>>> > not an issue or bug at all, there is nothing wrong with the existing
>>>> > code.
>>>> >
>>>>
>>>> I'm starting to regret that I ever accepted the original patch :-(
>>>>
>>>> -- Steve
>>>
>>>Okay, I hadn't looked at the previous conversations because this 
>>>change is
>>>in the upstream and in debian...
>>
>>Upstream is fine, it's a valid change so that people don't keep sending
>>the crazy patch over and over.
>>
>>Debian is just cargo-culting the thing and should probably drop it 
>>as it
>>keeps coming back to me every 3 months or so, and I have to reject it
>>again :(
>>
>>thanks,
>>
>>greg k-h
>
>Why didn't you follow the upstream or add a comment "no change for 
>fake CVE-2017-0605" to break the debian patch ?

This change is fine upstream, it doesn't even mention that CVE. It just
doesn't actually fix anything, so it doesn't belong in stable (nor is it
tagged for stable).

--
Thanks,
Sasha

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Greg KH]

On Mon, Dec 17, 2018 at 08:42:38PM +0100, Loic wrote:
> Le 2018-12-17 09:19, Greg KH a �crit�:
> > On Sun, Dec 16, 2018 at 09:08:20PM +0100, Loic wrote:
> > > Le 2018-12-16 20:27, Steven Rostedt a �crit�:
> > > > On Sun, 16 Dec 2018 09:52:33 +0100
> > > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > > On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
> > > > > > Hello,
> > > > > >
> > > > > > Please picked up this patch for linux 4.4 and 4.9.
> > > > > > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
> > > > >
> > > > > It was rejected as a CVE for a good reason, and that reason is also
> > > > > why
> > > > > I refused to add it to the stable kernel releases.  In short, this is
> > > > > not an issue or bug at all, there is nothing wrong with the existing
> > > > > code.
> > > > >
> > > >
> > > > I'm starting to regret that I ever accepted the original patch :-(
> > > >
> > > > -- Steve
> > > 
> > > Okay, I hadn't looked at the previous conversations because this
> > > change is
> > > in the upstream and in debian...
> > 
> > Upstream is fine, it's a valid change so that people don't keep sending
> > the crazy patch over and over.
> > 
> > Debian is just cargo-culting the thing and should probably drop it as it
> > keeps coming back to me every 3 months or so, and I have to reject it
> > again :(
> > 
> > thanks,
> > 
> > greg k-h
> 
> Why didn't you follow the upstream or add a comment "no change for fake
> CVE-2017-0605" to break the debian patch ?

How can I change upstream?  The commit can not be changed once it is
merged.

greg k-h

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

On Mon, 2018-12-17 at 20:42 +0100, Loic wrote:
> Le 2018-12-17 09:19, Greg KH a écrit :
> > On Sun, Dec 16, 2018 at 09:08:20PM +0100, Loic wrote:
> > > Le 2018-12-16 20:27, Steven Rostedt a écrit :
> > > > On Sun, 16 Dec 2018 09:52:33 +0100
> > > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > 
> > > > > On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > Please picked up this patch for linux 4.4 and 4.9.
> > > > > > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
> > > > > 
> > > > > It was rejected as a CVE for a good reason, and that reason is also
> > > > > why
> > > > > I refused to add it to the stable kernel releases.  In short, this is
> > > > > not an issue or bug at all, there is nothing wrong with the existing
> > > > > code.
> > > > > 
> > > > 
> > > > I'm starting to regret that I ever accepted the original patch :-(
> > > > 
> > > > -- Steve
> > > 
> > > Okay, I hadn't looked at the previous conversations because this 
> > > change is
> > > in the upstream and in debian...
> > 
> > Upstream is fine, it's a valid change so that people don't keep sending
> > the crazy patch over and over.
> > 
> > Debian is just cargo-culting the thing and should probably drop it as 
> > it
> > keeps coming back to me every 3 months or so, and I have to reject it
> > again :(
> > 
> > thanks,
> > 
> > greg k-h
> 
> Why didn't you follow the upstream or add a comment "no change for fake 
> CVE-2017-0605" to break the debian patch ?
>
> In short, I accuse the Debian kernel team in my defense, it's up to them 
> to buy you a beer :)

I was always sceptical about this CVE and commented to that effect in
<https://salsa.debian.org/kernel-team/kernel-sec/raw/master/retired/CVE-2017-0605>.
But the upstream "fix" also looked safe to apply just in case there was
something I was missing...

As it's causing confusion I can drop the patch from Debian now.

Ben.

-- 
Ben Hutchings
Anthony's Law of Force: Don't force it, get a larger hammer.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()

[Loic]

Le 2018-12-17 22:05, Greg KH a écrit :
> On Mon, Dec 17, 2018 at 08:42:38PM +0100, Loic wrote:
>> Le 2018-12-17 09:19, Greg KH a écrit :
>> > On Sun, Dec 16, 2018 at 09:08:20PM +0100, Loic wrote:
>> > > Le 2018-12-16 20:27, Steven Rostedt a écrit :
>> > > > On Sun, 16 Dec 2018 09:52:33 +0100
>> > > > Greg KH <gregkh@linuxfoundation.org> wrote:
>> > > >
>> > > > > On Sat, Dec 15, 2018 at 06:25:37PM +0100, Loic wrote:
>> > > > > > Hello,
>> > > > > >
>> > > > > > Please picked up this patch for linux 4.4 and 4.9.
>> > > > > > This fixes CVE-2017-0605 (Rejected?). Tested in Debian ;)
>> > > > >
>> > > > > It was rejected as a CVE for a good reason, and that reason is also
>> > > > > why
>> > > > > I refused to add it to the stable kernel releases.  In short, this is
>> > > > > not an issue or bug at all, there is nothing wrong with the existing
>> > > > > code.
>> > > > >
>> > > >
>> > > > I'm starting to regret that I ever accepted the original patch :-(
>> > > >
>> > > > -- Steve
>> > >
>> > > Okay, I hadn't looked at the previous conversations because this
>> > > change is
>> > > in the upstream and in debian...
>> >
>> > Upstream is fine, it's a valid change so that people don't keep sending
>> > the crazy patch over and over.
>> >
>> > Debian is just cargo-culting the thing and should probably drop it as it
>> > keeps coming back to me every 3 months or so, and I have to reject it
>> > again :(
>> >
>> > thanks,
>> >
>> > greg k-h
>> 
>> Why didn't you follow the upstream or add a comment "no change for 
>> fake
>> CVE-2017-0605" to break the debian patch ?
> 
> How can I change upstream?  The commit can not be changed once it is
> merged.
> 
> greg k-h

Sorry for my English.

No, I wanted to say a comment in stable to prevent this patch from being 
easily applied without reading the "fake CVE" comment.
This avoids some upstream commit arriving on stable.

Sorry for the waste of time. Thank you.

> I was always sceptical about this CVE and commented to that effect in
> <https://salsa.debian.org/kernel-team/kernel-sec/raw/master/retired/CVE-2017-0605>.
> But the upstream "fix" also looked safe to apply just in case there was
> something I was missing...
> 
> As it's causing confusion I can drop the patch from Debian now.
> 
> Ben.

Thank you very much.

-- 
Best regards,

Loic