WTF: patch "[PATCH] seccomp: make function __get_seccomp_filter static" was seriously submitted to be applied to the 4.13-stable tree?

WTF: patch "[PATCH] seccomp: make function __get_seccomp_filter static" was seriously submitted to be applied to the 4.13-stable tree?

[gregkh]

The patch below was submitted to be applied to the 4.13-stable tree.

I fail to see how this patch meets the stable kernel rules as found at
Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to 
<stable@vger.kernel.org> and let me know why this patch should be
applied.  Otherwise, it is now dropped from my patch queues, never to be
seen again.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From 084f5601c357e4ee59cf0712200d3f5c4710ba40 Mon Sep 17 00:00:00 2001
From: Colin Ian King <colin.king@canonical.com>
Date: Fri, 29 Sep 2017 14:26:48 +0100
Subject: [PATCH] seccomp: make function __get_seccomp_filter static

The function __get_seccomp_filter is local to the source and does
not need to be in global scope, so make it static.

Cleans up sparse warning:
symbol '__get_seccomp_filter' was not declared. Should it be static?

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Fixes: 66a733ea6b61 ("seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter()")
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index bb3a38005b9c..0ae832e13b97 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -473,7 +473,7 @@ static long seccomp_attach_filter(unsigned int flags,
 	return 0;
 }
 
-void __get_seccomp_filter(struct seccomp_filter *filter)
+static void __get_seccomp_filter(struct seccomp_filter *filter)
 {
 	/* Reference count is bounded by the number of total processes. */
 	refcount_inc(&filter->usage);

Re: WTF: patch "[PATCH] seccomp: make function __get_seccomp_filter static" was seriously submitted to be applied to the 4.13-stable tree?

[Colin Ian King]

On 12/10/17 12:39, gregkh@linuxfoundation.org wrote:
> The patch below was submitted to be applied to the 4.13-stable tree.
> 
> I fail to see how this patch meets the stable kernel rules as found at
> Documentation/process/stable-kernel-rules.rst.
> 
> I could be totally wrong, and if so, please respond to 
> <stable@vger.kernel.org> and let me know why this patch should be
> applied.  Otherwise, it is now dropped from my patch queues, never to be
> seen again.

I'm fairly sure my original patch didn't cc stable, so not sure why it
ended up here either.

Colin

> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From 084f5601c357e4ee59cf0712200d3f5c4710ba40 Mon Sep 17 00:00:00 2001
> From: Colin Ian King <colin.king@canonical.com>
> Date: Fri, 29 Sep 2017 14:26:48 +0100
> Subject: [PATCH] seccomp: make function __get_seccomp_filter static
> 
> The function __get_seccomp_filter is local to the source and does
> not need to be in global scope, so make it static.
> 
> Cleans up sparse warning:
> symbol '__get_seccomp_filter' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Fixes: 66a733ea6b61 ("seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index bb3a38005b9c..0ae832e13b97 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -473,7 +473,7 @@ static long seccomp_attach_filter(unsigned int flags,
>  	return 0;
>  }
>  
> -void __get_seccomp_filter(struct seccomp_filter *filter)
> +static void __get_seccomp_filter(struct seccomp_filter *filter)
>  {
>  	/* Reference count is bounded by the number of total processes. */
>  	refcount_inc(&filter->usage);
> 

Re: WTF: patch "[PATCH] seccomp: make function __get_seccomp_filter static" was seriously submitted to be applied to the 4.13-stable tree?

[Kees Cook]

On Thu, Oct 12, 2017 at 4:45 AM, Colin Ian King
<colin.king@canonical.com> wrote:
> On 12/10/17 12:39, gregkh@linuxfoundation.org wrote:
>> The patch below was submitted to be applied to the 4.13-stable tree.
>>
>> I fail to see how this patch meets the stable kernel rules as found at
>> Documentation/process/stable-kernel-rules.rst.
>>
>> I could be totally wrong, and if so, please respond to
>> <stable@vger.kernel.org> and let me know why this patch should be
>> applied.  Otherwise, it is now dropped from my patch queues, never to be
>> seen again.
>
> I'm fairly sure my original patch didn't cc stable, so not sure why it
> ended up here either.
>
> Colin
>
>>
>> thanks,
>>
>> greg k-h
>>
>> ------------------ original commit in Linus's tree ------------------
>>
>> From 084f5601c357e4ee59cf0712200d3f5c4710ba40 Mon Sep 17 00:00:00 2001
>> From: Colin Ian King <colin.king@canonical.com>
>> Date: Fri, 29 Sep 2017 14:26:48 +0100
>> Subject: [PATCH] seccomp: make function __get_seccomp_filter static
>>
>> The function __get_seccomp_filter is local to the source and does
>> not need to be in global scope, so make it static.
>>
>> Cleans up sparse warning:
>> symbol '__get_seccomp_filter' was not declared. Should it be static?
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> Fixes: 66a733ea6b61 ("seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter()")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index bb3a38005b9c..0ae832e13b97 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -473,7 +473,7 @@ static long seccomp_attach_filter(unsigned int flags,
>>       return 0;
>>  }
>>
>> -void __get_seccomp_filter(struct seccomp_filter *filter)
>> +static void __get_seccomp_filter(struct seccomp_filter *filter)
>>  {
>>       /* Reference count is bounded by the number of total processes. */
>>       refcount_inc(&filter->usage);
>>

I added this to -stable because the prior patch (66a733ea6b61) that
went to stable introduced a regression for Sparse. Is this not okay?

-Kees

-- 
Kees Cook
Pixel Security

Re: WTF: patch "[PATCH] seccomp: make function __get_seccomp_filter static" was seriously submitted to be applied to the 4.13-stable tree?

[Colin Ian King]

On 12/10/17 15:18, Kees Cook wrote:
> On Thu, Oct 12, 2017 at 4:45 AM, Colin Ian King
> <colin.king@canonical.com> wrote:
>> On 12/10/17 12:39, gregkh@linuxfoundation.org wrote:
>>> The patch below was submitted to be applied to the 4.13-stable tree.
>>>
>>> I fail to see how this patch meets the stable kernel rules as found at
>>> Documentation/process/stable-kernel-rules.rst.
>>>
>>> I could be totally wrong, and if so, please respond to
>>> <stable@vger.kernel.org> and let me know why this patch should be
>>> applied.  Otherwise, it is now dropped from my patch queues, never to be
>>> seen again.
>>
>> I'm fairly sure my original patch didn't cc stable, so not sure why it
>> ended up here either.
>>
>> Colin
>>
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>> ------------------ original commit in Linus's tree ------------------
>>>
>>> From 084f5601c357e4ee59cf0712200d3f5c4710ba40 Mon Sep 17 00:00:00 2001
>>> From: Colin Ian King <colin.king@canonical.com>
>>> Date: Fri, 29 Sep 2017 14:26:48 +0100
>>> Subject: [PATCH] seccomp: make function __get_seccomp_filter static
>>>
>>> The function __get_seccomp_filter is local to the source and does
>>> not need to be in global scope, so make it static.
>>>
>>> Cleans up sparse warning:
>>> symbol '__get_seccomp_filter' was not declared. Should it be static?
>>>
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> Fixes: 66a733ea6b61 ("seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter()")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>
>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>> index bb3a38005b9c..0ae832e13b97 100644
>>> --- a/kernel/seccomp.c
>>> +++ b/kernel/seccomp.c
>>> @@ -473,7 +473,7 @@ static long seccomp_attach_filter(unsigned int flags,
>>>       return 0;
>>>  }
>>>
>>> -void __get_seccomp_filter(struct seccomp_filter *filter)
>>> +static void __get_seccomp_filter(struct seccomp_filter *filter)
>>>  {
>>>       /* Reference count is bounded by the number of total processes. */
>>>       refcount_inc(&filter->usage);
>>>
> 
> I added this to -stable because the prior patch (66a733ea6b61) that
> went to stable introduced a regression for Sparse. Is this not okay?
> 
> -Kees
> 
I don't think it's a regression per-se, it's just a warning found by
static analysis tools and it won't show up in normal gcc builds.

Colin

Re: WTF: patch "[PATCH] seccomp: make function __get_seccomp_filter static" was seriously submitted to be applied to the 4.13-stable tree?

[Kees Cook]

On Thu, Oct 12, 2017 at 7:20 AM, Colin Ian King
<colin.king@canonical.com> wrote:
> On 12/10/17 15:18, Kees Cook wrote:
>> On Thu, Oct 12, 2017 at 4:45 AM, Colin Ian King
>> <colin.king@canonical.com> wrote:
>>> On 12/10/17 12:39, gregkh@linuxfoundation.org wrote:
>>>> The patch below was submitted to be applied to the 4.13-stable tree.
>>>>
>>>> I fail to see how this patch meets the stable kernel rules as found at
>>>> Documentation/process/stable-kernel-rules.rst.
>>>>
>>>> I could be totally wrong, and if so, please respond to
>>>> <stable@vger.kernel.org> and let me know why this patch should be
>>>> applied.  Otherwise, it is now dropped from my patch queues, never to be
>>>> seen again.
>>>
>>> I'm fairly sure my original patch didn't cc stable, so not sure why it
>>> ended up here either.
>>>
>>> Colin
>>>
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>>
>>>> ------------------ original commit in Linus's tree ------------------
>>>>
>>>> From 084f5601c357e4ee59cf0712200d3f5c4710ba40 Mon Sep 17 00:00:00 2001
>>>> From: Colin Ian King <colin.king@canonical.com>
>>>> Date: Fri, 29 Sep 2017 14:26:48 +0100
>>>> Subject: [PATCH] seccomp: make function __get_seccomp_filter static
>>>>
>>>> The function __get_seccomp_filter is local to the source and does
>>>> not need to be in global scope, so make it static.
>>>>
>>>> Cleans up sparse warning:
>>>> symbol '__get_seccomp_filter' was not declared. Should it be static?
>>>>
>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>> Fixes: 66a733ea6b61 ("seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter()")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>
>>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>>> index bb3a38005b9c..0ae832e13b97 100644
>>>> --- a/kernel/seccomp.c
>>>> +++ b/kernel/seccomp.c
>>>> @@ -473,7 +473,7 @@ static long seccomp_attach_filter(unsigned int flags,
>>>>       return 0;
>>>>  }
>>>>
>>>> -void __get_seccomp_filter(struct seccomp_filter *filter)
>>>> +static void __get_seccomp_filter(struct seccomp_filter *filter)
>>>>  {
>>>>       /* Reference count is bounded by the number of total processes. */
>>>>       refcount_inc(&filter->usage);
>>>>
>>
>> I added this to -stable because the prior patch (66a733ea6b61) that
>> went to stable introduced a regression for Sparse. Is this not okay?
>>
> I don't think it's a regression per-se, it's just a warning found by
> static analysis tools and it won't show up in normal gcc builds.

If this isn't okay, I'm fine to drop it from -stable. Sorry for the
noise Greg! I was considering it a build regression, since there are
people running analyzers on -stable, etc.

-Kees

-- 
Kees Cook
Pixel Security