b43/leds: Ensure NUL-termination of LED name string

b43/leds: Ensure NUL-termination of LED name string

[Michael Büsch]

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

strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.

Signed-off-by: Michael Buesch <m@bues.ch>
Cc: stable@vger.kernel.org
---
 drivers/net/wireless/broadcom/b43/leds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/leds.c b/drivers/net/wireless/broadcom/b43/leds.c
index cb987c2ecc6b..87131f663292 100644
--- a/drivers/net/wireless/broadcom/b43/leds.c
+++ b/drivers/net/wireless/broadcom/b43/leds.c
@@ -131,7 +131,7 @@ static int b43_register_led(struct b43_wldev *dev, struct b43_led *led,
 	led->wl = dev->wl;
 	led->index = led_index;
 	led->activelow = activelow;
-	strncpy(led->name, name, sizeof(led->name));
+	strlcpy(led->name, name, sizeof(led->name));
 	atomic_set(&led->state, 0);
 
 	led->led_dev.name = led->name;

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: b43/leds: Ensure NUL-termination of LED name string

[Kalle Valo]

TWljaGFlbCBCw7xzY2ggPG1AYnVlcy5jaD4gd3JpdGVzOg0KDQo+IHN0cm5jcHkgbWlnaHQgbm90
IE5VTC10ZXJtaW5hdGUgdGhlIHN0cmluZywgaWYgdGhlIG5hbWUgZXF1YWxzIHRoZSBidWZmZXIg
c2l6ZS4NCj4gVXNlIHN0cmxjcHkgaW5zdGVhZC4NCj4NCj4gU2lnbmVkLW9mZi1ieTogTWljaGFl
bCBCdWVzY2ggPG1AYnVlcy5jaD4NCj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcNCg0KVGhp
cyBpcyB3ZWlyZCwgd2l0aCBhbGwgdGhlIHBhdGNoZXMgeW91IHN1Ym1pdHRlZCBsYXN0IHdlZWsg
SSBnZXQgdGhpcw0KaWYgSSBkb3dubG9hZCB0aGUgcGF0Y2ggZnJvbSBwYXRjaHdvcms6DQoNCiQg
Z2l0IGFtIC1zIDEubWJveA0KUGF0Y2ggaXMgZW1wdHkuIFdhcyBpdCBzcGxpdCB3cm9uZz8NCg0K
QnV0IGlmIEkgZG93bmxvYWQgdGhlIHBhdGNoIGRpcmVjdGx5IGZyb20gbXkgSU1BUCBmb2xkZXIg
SSBoYXZlIG5vDQpwcm9ibGVtczoNCg0KJCBnaXQgYW0gLXMgMS5tYm94DQpBcHBseWluZzogYjQz
L2xlZHM6IEVuc3VyZSBOVUwtdGVybWluYXRpb24gb2YgTEVEIG5hbWUgc3RyaW5nDQoNClRoaXMg
aGFwcGVucyBldmVuIHdpdGhvdXQgbXkgY3VzdG9tIHBhdGNod29yayBzY3JpcHQgc28gdGhpcyBo
YXMNCnNvbWV0aGluZyB0byBkbyB3aXRoIHRoZSBwYXRjaHdvcmsgc2VydmVyLCBidXQgaXQncyBu
b3Qgb2J2aW91cyB0byBtZQ0Kd2hhdCB0cmlnZ2VycyBpdC4gSUlSQyBJIGhhdmUgbm90IHNlZW4g
YW55dGhpbmcgbGlrZSB0aGlzIGJlZm9yZS4gSXQNCnNlZW1zIHRoYXQgeW91IGRpZG4ndCB1c2Ug
Z2l0LXNlbmQtZW1haWwsIEkgc3Ryb25nbHkgc3VnZ2VzdCB0byB1c2UgdGhhdA0KanVzdCB0byBh
dm9pZCBwcm9ibGVtcyBsaWtlIHRoaXMuDQoNCkFueXdheSwgYXBwbGllZCBtYW51YWxseToNCg0K
MmFhNjUwZDE5NTBmIGI0My9sZWRzOiBFbnN1cmUgTlVMLXRlcm1pbmF0aW9uIG9mIExFRCBuYW1l
IHN0cmluZw0KDQotLSANCkthbGxlIFZhbG8=

Re: b43/leds: Ensure NUL-termination of LED name string

[Jonas Gorski]

On 9 August 2018 at 17:28, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michael B=C3=BCsch <m@bues.ch> writes:
>
>> strncpy might not NUL-terminate the string, if the name equals the buffe=
r size.
>> Use strlcpy instead.
>>
>> Signed-off-by: Michael Buesch <m@bues.ch>
>> Cc: stable@vger.kernel.org
>
> This is weird, with all the patches you submitted last week I get this
> if I download the patch from patchwork:
>
> $ git am -s 1.mbox
> Patch is empty. Was it split wrong?
>
> But if I download the patch directly from my IMAP folder I have no
> problems:
>
> $ git am -s 1.mbox
> Applying: b43/leds: Ensure NUL-termination of LED name string
>
> This happens even without my custom patchwork script so this has
> something to do with the patchwork server, but it's not obvious to me
> what triggers it. IIRC I have not seen anything like this before. It
> seems that you didn't use git-send-email, I strongly suggest to use that
> just to avoid problems like this.

Looks like patchwork mishandles the pgp signature, the patchwork mbox has

> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>  boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/pgp-s=
ignature"

as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has

> Content-Type: multipart/signed; micalg=3Dpgp-sha512; boundary=3D"Sig_/EN9=
0ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/pgp-signature"
>
> --Sig_/EN90ciRq4eWXDUcnZABQ0Ak
> Content-Type: text/plain; charset=3DUS-ASCII
> Content-Transfer-Encoding: quoted-printable

When I remove the Content-Type: line(s) from the mbox from patchwork,
git recognises it again as a patch. I guess git am ignores everything
until the boundary, which got dropped by patchwork, so it never finds
the actual patch.


Regards
Jonas

Re: b43/leds: Ensure NUL-termination of LED name string

[Kalle Valo]

Jonas Gorski <jonas.gorski@gmail.com> writes:

> On 9 August 2018 at 17:28, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michael B=C3=BCsch <m@bues.ch> writes:
>>
>>> strncpy might not NUL-terminate the string, if the name equals the buff=
er size.
>>> Use strlcpy instead.
>>>
>>> Signed-off-by: Michael Buesch <m@bues.ch>
>>> Cc: stable@vger.kernel.org
>>
>> This is weird, with all the patches you submitted last week I get this
>> if I download the patch from patchwork:
>>
>> $ git am -s 1.mbox
>> Patch is empty. Was it split wrong?
>>
>> But if I download the patch directly from my IMAP folder I have no
>> problems:
>>
>> $ git am -s 1.mbox
>> Applying: b43/leds: Ensure NUL-termination of LED name string
>>
>> This happens even without my custom patchwork script so this has
>> something to do with the patchwork server, but it's not obvious to me
>> what triggers it. IIRC I have not seen anything like this before. It
>> seems that you didn't use git-send-email, I strongly suggest to use that
>> just to avoid problems like this.
>
> Looks like patchwork mishandles the pgp signature, the patchwork mbox has
>
>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>  boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/pgp-=
signature"
>
> as the only content-type (and the boundary is nowhere to be found),
> while the one in my inbox has
>
>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>> boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
>> protocol=3D"application/pgp-signature"
>>
>> --Sig_/EN90ciRq4eWXDUcnZABQ0Ak
>> Content-Type: text/plain; charset=3DUS-ASCII
>> Content-Transfer-Encoding: quoted-printable
>
> When I remove the Content-Type: line(s) from the mbox from patchwork,
> git recognises it again as a patch. I guess git am ignores everything
> until the boundary, which got dropped by patchwork, so it never finds
> the actual patch.

Awesome, thanks for debugging this! It would be great if someone could
report this to the patchwork maintainers, I don't have the time right
now.

--=20
Kalle Valo

Re: b43/leds: Ensure NUL-termination of LED name string

[Jonas Gorski]

On 9 August 2018 at 18:15, Kalle Valo <kvalo@codeaurora.org> wrote:
> Jonas Gorski <jonas.gorski@gmail.com> writes:
>
>> On 9 August 2018 at 17:28, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>> Michael B=C3=BCsch <m@bues.ch> writes:
>>>
>>>> strncpy might not NUL-terminate the string, if the name equals the buf=
fer size.
>>>> Use strlcpy instead.
>>>>
>>>> Signed-off-by: Michael Buesch <m@bues.ch>
>>>> Cc: stable@vger.kernel.org
>>>
>>> This is weird, with all the patches you submitted last week I get this
>>> if I download the patch from patchwork:
>>>
>>> $ git am -s 1.mbox
>>> Patch is empty. Was it split wrong?
>>>
>>> But if I download the patch directly from my IMAP folder I have no
>>> problems:
>>>
>>> $ git am -s 1.mbox
>>> Applying: b43/leds: Ensure NUL-termination of LED name string
>>>
>>> This happens even without my custom patchwork script so this has
>>> something to do with the patchwork server, but it's not obvious to me
>>> what triggers it. IIRC I have not seen anything like this before. It
>>> seems that you didn't use git-send-email, I strongly suggest to use tha=
t
>>> just to avoid problems like this.
>>
>> Looks like patchwork mishandles the pgp signature, the patchwork mbox ha=
s
>>
>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>  boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/pgp=
-signature"
>>
>> as the only content-type (and the boundary is nowhere to be found),
>> while the one in my inbox has
>>
>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>> boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
>>> protocol=3D"application/pgp-signature"
>>>
>>> --Sig_/EN90ciRq4eWXDUcnZABQ0Ak
>>> Content-Type: text/plain; charset=3DUS-ASCII
>>> Content-Transfer-Encoding: quoted-printable
>>
>> When I remove the Content-Type: line(s) from the mbox from patchwork,
>> git recognises it again as a patch. I guess git am ignores everything
>> until the boundary, which got dropped by patchwork, so it never finds
>> the actual patch.
>
> Awesome, thanks for debugging this! It would be great if someone could
> report this to the patchwork maintainers, I don't have the time right
> now.

Silly question, but who *are* the maintainers? I just spend several
minutes on https://patchwork.kernel.org/ and google, and failed to
find any contact information.


Regards
Jonas

Re: b43/leds: Ensure NUL-termination of LED name string

[Kalle Valo]

Jonas Gorski <jonas.gorski@gmail.com> writes:

> On 9 August 2018 at 18:15, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Jonas Gorski <jonas.gorski@gmail.com> writes:
>>
>>> On 9 August 2018 at 17:28, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>>> Michael B=C3=BCsch <m@bues.ch> writes:
>>>>
>>>>> strncpy might not NUL-terminate the string, if the name equals the bu=
ffer size.
>>>>> Use strlcpy instead.
>>>>>
>>>>> Signed-off-by: Michael Buesch <m@bues.ch>
>>>>> Cc: stable@vger.kernel.org
>>>>
>>>> This is weird, with all the patches you submitted last week I get this
>>>> if I download the patch from patchwork:
>>>>
>>>> $ git am -s 1.mbox
>>>> Patch is empty. Was it split wrong?
>>>>
>>>> But if I download the patch directly from my IMAP folder I have no
>>>> problems:
>>>>
>>>> $ git am -s 1.mbox
>>>> Applying: b43/leds: Ensure NUL-termination of LED name string
>>>>
>>>> This happens even without my custom patchwork script so this has
>>>> something to do with the patchwork server, but it's not obvious to me
>>>> what triggers it. IIRC I have not seen anything like this before. It
>>>> seems that you didn't use git-send-email, I strongly suggest to use th=
at
>>>> just to avoid problems like this.
>>>
>>> Looks like patchwork mishandles the pgp signature, the patchwork mbox h=
as
>>>
>>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>>  boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/pg=
p-signature"
>>>
>>> as the only content-type (and the boundary is nowhere to be found),
>>> while the one in my inbox has
>>>
>>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>> boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
>>>> protocol=3D"application/pgp-signature"
>>>>
>>>> --Sig_/EN90ciRq4eWXDUcnZABQ0Ak
>>>> Content-Type: text/plain; charset=3DUS-ASCII
>>>> Content-Transfer-Encoding: quoted-printable
>>>
>>> When I remove the Content-Type: line(s) from the mbox from patchwork,
>>> git recognises it again as a patch. I guess git am ignores everything
>>> until the boundary, which got dropped by patchwork, so it never finds
>>> the actual patch.
>>
>> Awesome, thanks for debugging this! It would be great if someone could
>> report this to the patchwork maintainers, I don't have the time right
>> now.
>
> Silly question, but who *are* the maintainers? I just spend several
> minutes on https://patchwork.kernel.org/ and google, and failed to
> find any contact information.

Not a silly question at all. I'm not sure what's the preferred way to
report bugs but at least I found this:

http://jk.ozlabs.org/projects/patchwork/

I guess they use the github tracker:

https://github.com/getpatchwork/patchwork/issues/

--=20
Kalle Valo

Re: b43/leds: Ensure NUL-termination of LED name string

[Jonas Gorski]

On 9 August 2018 at 19:01, Kalle Valo <kvalo@codeaurora.org> wrote:
> Jonas Gorski <jonas.gorski@gmail.com> writes:
>
>> On 9 August 2018 at 18:15, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Jonas Gorski <jonas.gorski@gmail.com> writes:
>>>
>>>> On 9 August 2018 at 17:28, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>>>> Michael B=C3=BCsch <m@bues.ch> writes:
>>>>>
>>>>>> strncpy might not NUL-terminate the string, if the name equals the b=
uffer size.
>>>>>> Use strlcpy instead.
>>>>>>
>>>>>> Signed-off-by: Michael Buesch <m@bues.ch>
>>>>>> Cc: stable@vger.kernel.org
>>>>>
>>>>> This is weird, with all the patches you submitted last week I get thi=
s
>>>>> if I download the patch from patchwork:
>>>>>
>>>>> $ git am -s 1.mbox
>>>>> Patch is empty. Was it split wrong?
>>>>>
>>>>> But if I download the patch directly from my IMAP folder I have no
>>>>> problems:
>>>>>
>>>>> $ git am -s 1.mbox
>>>>> Applying: b43/leds: Ensure NUL-termination of LED name string
>>>>>
>>>>> This happens even without my custom patchwork script so this has
>>>>> something to do with the patchwork server, but it's not obvious to me
>>>>> what triggers it. IIRC I have not seen anything like this before. It
>>>>> seems that you didn't use git-send-email, I strongly suggest to use t=
hat
>>>>> just to avoid problems like this.
>>>>
>>>> Looks like patchwork mishandles the pgp signature, the patchwork mbox =
has
>>>>
>>>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>>>  boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/p=
gp-signature"
>>>>
>>>> as the only content-type (and the boundary is nowhere to be found),
>>>> while the one in my inbox has
>>>>
>>>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>>> boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
>>>>> protocol=3D"application/pgp-signature"
>>>>>
>>>>> --Sig_/EN90ciRq4eWXDUcnZABQ0Ak
>>>>> Content-Type: text/plain; charset=3DUS-ASCII
>>>>> Content-Transfer-Encoding: quoted-printable
>>>>
>>>> When I remove the Content-Type: line(s) from the mbox from patchwork,
>>>> git recognises it again as a patch. I guess git am ignores everything
>>>> until the boundary, which got dropped by patchwork, so it never finds
>>>> the actual patch.
>>>
>>> Awesome, thanks for debugging this! It would be great if someone could
>>> report this to the patchwork maintainers, I don't have the time right
>>> now.
>>
>> Silly question, but who *are* the maintainers? I just spend several
>> minutes on https://patchwork.kernel.org/ and google, and failed to
>> find any contact information.
>
> Not a silly question at all. I'm not sure what's the preferred way to
> report bugs but at least I found this:
>
> http://jk.ozlabs.org/projects/patchwork/
>
> I guess they use the github tracker:
>
> https://github.com/getpatchwork/patchwork/issues/

Going through the issues, I guess this is already fixed in master with

https://github.com/getpatchwork/patchwork/commit/e27ff061dc01e51967a978884a=
5c59152863ab9c

and queued for the next 2.1 release (or not? the stable/2.1 branch
also contains a version bump to 2.2 ... )

I have still no idea who runs the patchwork instance at
patchwork.kernel.org though.


Regards
Jonas

Re: b43/leds: Ensure NUL-termination of LED name string

[Kalle Valo]

Jonas Gorski <jonas.gorski@gmail.com> writes:

> On 9 August 2018 at 19:01, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Jonas Gorski <jonas.gorski@gmail.com> writes:
>>
>>> On 9 August 2018 at 18:15, Kalle Valo <kvalo@codeaurora.org> wrote:
>>>> Jonas Gorski <jonas.gorski@gmail.com> writes:
>>>>
>>>>> On 9 August 2018 at 17:28, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>>>>> Michael B=C3=BCsch <m@bues.ch> writes:
>>>>>>
>>>>>>> strncpy might not NUL-terminate the string, if the name equals the =
buffer size.
>>>>>>> Use strlcpy instead.
>>>>>>>
>>>>>>> Signed-off-by: Michael Buesch <m@bues.ch>
>>>>>>> Cc: stable@vger.kernel.org
>>>>>>
>>>>>> This is weird, with all the patches you submitted last week I get th=
is
>>>>>> if I download the patch from patchwork:
>>>>>>
>>>>>> $ git am -s 1.mbox
>>>>>> Patch is empty. Was it split wrong?
>>>>>>
>>>>>> But if I download the patch directly from my IMAP folder I have no
>>>>>> problems:
>>>>>>
>>>>>> $ git am -s 1.mbox
>>>>>> Applying: b43/leds: Ensure NUL-termination of LED name string
>>>>>>
>>>>>> This happens even without my custom patchwork script so this has
>>>>>> something to do with the patchwork server, but it's not obvious to me
>>>>>> what triggers it. IIRC I have not seen anything like this before. It
>>>>>> seems that you didn't use git-send-email, I strongly suggest to use =
that
>>>>>> just to avoid problems like this.
>>>>>
>>>>> Looks like patchwork mishandles the pgp signature, the patchwork mbox=
 has
>>>>>
>>>>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>>>>  boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol=3D"application/=
pgp-signature"
>>>>>
>>>>> as the only content-type (and the boundary is nowhere to be found),
>>>>> while the one in my inbox has
>>>>>
>>>>>> Content-Type: multipart/signed; micalg=3Dpgp-sha512;
>>>>>> boundary=3D"Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
>>>>>> protocol=3D"application/pgp-signature"
>>>>>>
>>>>>> --Sig_/EN90ciRq4eWXDUcnZABQ0Ak
>>>>>> Content-Type: text/plain; charset=3DUS-ASCII
>>>>>> Content-Transfer-Encoding: quoted-printable
>>>>>
>>>>> When I remove the Content-Type: line(s) from the mbox from patchwork,
>>>>> git recognises it again as a patch. I guess git am ignores everything
>>>>> until the boundary, which got dropped by patchwork, so it never finds
>>>>> the actual patch.
>>>>
>>>> Awesome, thanks for debugging this! It would be great if someone could
>>>> report this to the patchwork maintainers, I don't have the time right
>>>> now.
>>>
>>> Silly question, but who *are* the maintainers? I just spend several
>>> minutes on https://patchwork.kernel.org/ and google, and failed to
>>> find any contact information.
>>
>> Not a silly question at all. I'm not sure what's the preferred way to
>> report bugs but at least I found this:
>>
>> http://jk.ozlabs.org/projects/patchwork/
>>
>> I guess they use the github tracker:
>>
>> https://github.com/getpatchwork/patchwork/issues/
>
> Going through the issues, I guess this is already fixed in master with
>
> https://github.com/getpatchwork/patchwork/commit/e27ff061dc01e51967a97888=
4a5c59152863ab9c
>
> and queued for the next 2.1 release (or not? the stable/2.1 branch
> also contains a version bump to 2.2 ... )

Very nice, thanks so much for investigating this!

> I have still no idea who runs the patchwork instance at
> patchwork.kernel.org though.

The admins are Linux Foundation's IT group and one can file a ticket via
helpdesk@kernel.org But IMHO there's no need to do that, this is the
first time I'm seeing the bug and it can easily wait until
patchwork.kernel.org is updated anyway. As long as the issue is fixed
upstream.

--=20
Kalle Valo