Discussion:
b43/leds: Ensure NUL-termination of LED name string
Michael Büsch
2018-07-31 19:14:04 UTC
Permalink
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.

Signed-off-by: Michael Buesch <***@bues.ch>
Cc: ***@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
Kalle Valo
2018-08-09 15:28:08 UTC
Permalink
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
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.

Anyway, applied manually:

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

--
Kalle Valo
Jonas Gorski
2018-08-09 16:02:06 UTC
Permalink
Post by Kalle Valo
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
This is weird, with all the patches you submitted last week I get this
$ 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
$ 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
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512; boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
--Sig_/EN90ciRq4eWXDUcnZABQ0Ak
Content-Type: text/plain; charset=US-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
Kalle Valo
2018-08-09 16:15:22 UTC
Permalink
Post by Jonas Gorski
Post by Kalle Valo
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
This is weird, with all the patches you submitted last week I get this
$ 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
$ 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
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
protocol="application/pgp-signature"
--Sig_/EN90ciRq4eWXDUcnZABQ0Ak
Content-Type: text/plain; charset=US-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.
--
Kalle Valo
Jonas Gorski
2018-08-09 16:41:04 UTC
Permalink
Post by Kalle Valo
Post by Jonas Gorski
Post by Kalle Valo
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
This is weird, with all the patches you submitted last week I get this
$ 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
$ 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
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
protocol="application/pgp-signature"
--Sig_/EN90ciRq4eWXDUcnZABQ0Ak
Content-Type: text/plain; charset=US-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
Kalle Valo
2018-08-09 17:01:59 UTC
Permalink
Post by Jonas Gorski
Post by Kalle Valo
Post by Jonas Gorski
Post by Kalle Valo
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
This is weird, with all the patches you submitted last week I get this
$ 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
$ 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
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
protocol="application/pgp-signature"
--Sig_/EN90ciRq4eWXDUcnZABQ0Ak
Content-Type: text/plain; charset=US-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/
--
Kalle Valo
Jonas Gorski
2018-08-10 08:31:40 UTC
Permalink
Post by Kalle Valo
Post by Jonas Gorski
Post by Kalle Valo
Post by Jonas Gorski
Post by Kalle Valo
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
This is weird, with all the patches you submitted last week I get this
$ 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
$ 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
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
protocol="application/pgp-signature"
--Sig_/EN90ciRq4eWXDUcnZABQ0Ak
Content-Type: text/plain; charset=US-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
http://jk.ozlabs.org/projects/patchwork/
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/e27ff061dc01e51967a978884a5c59152863ab9c

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
Kalle Valo
2018-08-10 14:30:35 UTC
Permalink
Post by Jonas Gorski
Post by Kalle Valo
Post by Jonas Gorski
Post by Kalle Valo
Post by Jonas Gorski
Post by Kalle Valo
Post by Michael Büsch
strncpy might not NUL-terminate the string, if the name equals the buffer size.
Use strlcpy instead.
This is weird, with all the patches you submitted last week I get this
$ 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
$ 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
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak"; protocol="application/pgp-signature"
as the only content-type (and the boundary is nowhere to be found),
while the one in my inbox has
Post by Kalle Valo
Content-Type: multipart/signed; micalg=pgp-sha512;
boundary="Sig_/EN90ciRq4eWXDUcnZABQ0Ak";
protocol="application/pgp-signature"
--Sig_/EN90ciRq4eWXDUcnZABQ0Ak
Content-Type: text/plain; charset=US-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
http://jk.ozlabs.org/projects/patchwork/
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/e27ff061dc01e51967a978884a5c59152863ab9c
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!
Post by Jonas Gorski
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
***@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.
--
Kalle Valo
Loading...