* [PATCH] can: Use correct type in sizeof() in nla_put()
@ 2015-10-30 12:48 Marek Vasut
2015-10-30 13:40 ` Marc Kleine-Budde
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-10-30 12:48 UTC (permalink / raw)
To: linux-can; +Cc: Marek Vasut, Wolfgang Grandegger, Marc Kleine-Budde, netdev
The sizeof() is invoked on an incorrect variable, likely due to some
copy-paste error, and this might result in memory corruption. Fix this.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: netdev@vger.kernel.org
---
drivers/net/can/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
NOTE: I only compile-tested this.
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index aede704..141c2a4 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -915,7 +915,7 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put(skb, IFLA_CAN_BITTIMING_CONST,
sizeof(*priv->bittiming_const), priv->bittiming_const)) ||
- nla_put(skb, IFLA_CAN_CLOCK, sizeof(cm), &priv->clock) ||
+ nla_put(skb, IFLA_CAN_CLOCK, sizeof(priv->clock), &priv->clock) ||
nla_put_u32(skb, IFLA_CAN_STATE, state) ||
nla_put(skb, IFLA_CAN_CTRLMODE, sizeof(cm), &cm) ||
nla_put_u32(skb, IFLA_CAN_RESTART_MS, priv->restart_ms) ||
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 12:48 [PATCH] can: Use correct type in sizeof() in nla_put() Marek Vasut
@ 2015-10-30 13:40 ` Marc Kleine-Budde
2015-10-30 14:01 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Marc Kleine-Budde @ 2015-10-30 13:40 UTC (permalink / raw)
To: Marek Vasut, linux-can; +Cc: Wolfgang Grandegger, netdev
[-- Attachment #1: Type: text/plain, Size: 700 bytes --]
On 10/30/2015 01:48 PM, Marek Vasut wrote:
> The sizeof() is invoked on an incorrect variable, likely due to some
> copy-paste error, and this might result in memory corruption. Fix this.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Grandegger <wg@grandegger.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: netdev@vger.kernel.org
Applies to can and added stable on Cc.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 13:40 ` Marc Kleine-Budde
@ 2015-10-30 14:01 ` Marek Vasut
2015-10-30 14:17 ` Marc Kleine-Budde
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-10-30 14:01 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Wolfgang Grandegger, netdev
On Friday, October 30, 2015 at 02:40:26 PM, Marc Kleine-Budde wrote:
> On 10/30/2015 01:48 PM, Marek Vasut wrote:
> > The sizeof() is invoked on an incorrect variable, likely due to some
> > copy-paste error, and this might result in memory corruption. Fix this.
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Wolfgang Grandegger <wg@grandegger.com>
> > Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> > Cc: netdev@vger.kernel.org
>
> Applies to can and added stable on Cc.
Are you absolutelly positive this doesn't break kernel ABI please ?
I am a little worried there, since the size of can_clock and can_ctrlmode
structures differ.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 14:01 ` Marek Vasut
@ 2015-10-30 14:17 ` Marc Kleine-Budde
2015-10-30 14:24 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Marc Kleine-Budde @ 2015-10-30 14:17 UTC (permalink / raw)
To: Marek Vasut; +Cc: linux-can, Wolfgang Grandegger, netdev
[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]
On 10/30/2015 03:01 PM, Marek Vasut wrote:
> On Friday, October 30, 2015 at 02:40:26 PM, Marc Kleine-Budde wrote:
>> On 10/30/2015 01:48 PM, Marek Vasut wrote:
>>> The sizeof() is invoked on an incorrect variable, likely due to some
>>> copy-paste error, and this might result in memory corruption. Fix this.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Wolfgang Grandegger <wg@grandegger.com>
>>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Cc: netdev@vger.kernel.org
>>
>> Applies to can and added stable on Cc.
>
> Are you absolutelly positive this doesn't break kernel ABI please ?
>
> I am a little worried there, since the size of can_clock and can_ctrlmode
> structures differ.
struct can_clock is a u32, see [1]
struct can_ctrlmode is 2 x u32.
in libsocketcan[2] it's accessed like this:
> memcpy(res,
> RTA_DATA(can_attr[IFLA_CAN_CLOCK]),
> sizeof(struct can_clock));
I think it should be ok.
Marc
[1]
http://lxr.free-electrons.com/source/include/uapi/linux/can/netlink.h#L61
[2]
http://git.pengutronix.de/?p=tools/libsocketcan.git;a=blob;f=src/libsocketcan.c;h=c97a28cca18054c8e63326eeb5a866b79344ebe2;hb=4ea9ec7cf37a0c52f2c39a13887aaad11042ef5c#l453
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 14:17 ` Marc Kleine-Budde
@ 2015-10-30 14:24 ` Marek Vasut
2015-10-30 14:53 ` Oliver Hartkopp
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-10-30 14:24 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Wolfgang Grandegger, netdev
On Friday, October 30, 2015 at 03:17:44 PM, Marc Kleine-Budde wrote:
> On 10/30/2015 03:01 PM, Marek Vasut wrote:
> > On Friday, October 30, 2015 at 02:40:26 PM, Marc Kleine-Budde wrote:
> >> On 10/30/2015 01:48 PM, Marek Vasut wrote:
> >>> The sizeof() is invoked on an incorrect variable, likely due to some
> >>> copy-paste error, and this might result in memory corruption. Fix this.
> >>>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Wolfgang Grandegger <wg@grandegger.com>
> >>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> >>> Cc: netdev@vger.kernel.org
> >>
> >> Applies to can and added stable on Cc.
> >
> > Are you absolutelly positive this doesn't break kernel ABI please ?
> >
> > I am a little worried there, since the size of can_clock and can_ctrlmode
> > structures differ.
>
> struct can_clock is a u32, see [1]
> struct can_ctrlmode is 2 x u32.
>
> in libsocketcan[2] it's accessed like this:
> > memcpy(res,
> >
> > RTA_DATA(can_attr[IFLA_CAN_CLOCK]),
> > sizeof(struct can_clock));
>
> I think it should be ok.
In that case, yes, it's good. Hopefully, noone wrote his own thing.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 14:24 ` Marek Vasut
@ 2015-10-30 14:53 ` Oliver Hartkopp
2015-10-30 15:33 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Oliver Hartkopp @ 2015-10-30 14:53 UTC (permalink / raw)
To: Marek Vasut, Marc Kleine-Budde; +Cc: linux-can, Wolfgang Grandegger, netdev
On 10/30/2015 03:24 PM, Marek Vasut wrote:
> On Friday, October 30, 2015 at 03:17:44 PM, Marc Kleine-Budde wrote:
>> On 10/30/2015 03:01 PM, Marek Vasut wrote:
>>> Are you absolutelly positive this doesn't break kernel ABI please ?
>>>
>>> I am a little worried there, since the size of can_clock and can_ctrlmode
>>> structures differ.
>>
>> struct can_clock is a u32, see [1]
>> struct can_ctrlmode is 2 x u32.
>>
>> in libsocketcan[2] it's accessed like this:
>>> memcpy(res,
>>>
>>> RTA_DATA(can_attr[IFLA_CAN_CLOCK]),
>>> sizeof(struct can_clock));
>>
>> I think it should be ok.
>
> In that case, yes, it's good. Hopefully, noone wrote his own thing.
>
Fortunately ip from iproute2 does it similary:
https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/tree/ip/iplink_can.c#n338
if (tb[IFLA_CAN_CLOCK]) {
struct can_clock *clock = RTA_DATA(tb[IFLA_CAN_CLOCK]);
fprintf(f, "\n clock %d", clock->freq);
}
As the clock is a read-only value kernel->userspace and nla_put creates its
own small ID/length information each time we are REALLY LUCKY that this fix
doesn't break the ABI in this case.
When can_clock would have been greater then can_ctrlmode we really had a
problem ...
Thanks for caching this!
Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 14:53 ` Oliver Hartkopp
@ 2015-10-30 15:33 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2015-10-30 15:33 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Marc Kleine-Budde, linux-can, Wolfgang Grandegger, netdev
On Friday, October 30, 2015 at 03:53:31 PM, Oliver Hartkopp wrote:
> On 10/30/2015 03:24 PM, Marek Vasut wrote:
> > On Friday, October 30, 2015 at 03:17:44 PM, Marc Kleine-Budde wrote:
> >> On 10/30/2015 03:01 PM, Marek Vasut wrote:
> >>> Are you absolutelly positive this doesn't break kernel ABI please ?
> >>>
> >>> I am a little worried there, since the size of can_clock and
> >>> can_ctrlmode structures differ.
> >>
> >> struct can_clock is a u32, see [1]
> >> struct can_ctrlmode is 2 x u32.
> >>
> >> in libsocketcan[2] it's accessed like this:
> >>> memcpy(res,
> >>>
> >>> RTA_DATA(can_attr[IFLA_CAN_CLOCK]),
> >>> sizeof(struct can_clock));
> >>
> >> I think it should be ok.
> >
> > In that case, yes, it's good. Hopefully, noone wrote his own thing.
>
> Fortunately ip from iproute2 does it similary:
>
> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/tree/i
> p/iplink_can.c#n338
>
>
> if (tb[IFLA_CAN_CLOCK]) {
> struct can_clock *clock = RTA_DATA(tb[IFLA_CAN_CLOCK]);
>
> fprintf(f, "\n clock %d", clock->freq);
> }
>
> As the clock is a read-only value kernel->userspace and nla_put creates its
> own small ID/length information each time we are REALLY LUCKY that this fix
> doesn't break the ABI in this case.
>
> When can_clock would have been greater then can_ctrlmode we really had a
> problem ...
>
> Thanks for caching this!
Yeah, I already had one leg in my asbestos trousers all right. Thanks for
double-checking this!
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* pull-request: can 2015-10-30
@ 2015-10-30 13:39 Marc Kleine-Budde
2015-10-30 13:39 ` [PATCH] can: Use correct type in sizeof() in nla_put() Marc Kleine-Budde
0 siblings, 1 reply; 8+ messages in thread
From: Marc Kleine-Budde @ 2015-10-30 13:39 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request for the upcoming v4.3 release.
Marek Vasut provides a patch to use the correct attrlen in the nla_put() in the
can_fill_info() function.
regards,
Marc
---
The following changes since commit 89bc7848a91bc99532f5c21b2885472ba710f249:
ipv6: protect mtu calculation of wrap-around and infinite loop by rounding issues (2015-10-29 07:01:50 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.3-20151030
for you to fetch changes up to 562b103a21974c2f9cd67514d110f918bb3e1796:
can: Use correct type in sizeof() in nla_put() (2015-10-30 14:33:26 +0100)
----------------------------------------------------------------
linux-can-fixes-for-4.3-20151030
----------------------------------------------------------------
Marek Vasut (1):
can: Use correct type in sizeof() in nla_put()
drivers/net/can/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] can: Use correct type in sizeof() in nla_put()
2015-10-30 13:39 pull-request: can 2015-10-30 Marc Kleine-Budde
@ 2015-10-30 13:39 ` Marc Kleine-Budde
0 siblings, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2015-10-30 13:39 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Marek Vasut, Wolfgang Grandegger,
linux-stable, Marc Kleine-Budde
From: Marek Vasut <marex@denx.de>
The sizeof() is invoked on an incorrect variable, likely due to some
copy-paste error, and this might result in memory corruption. Fix this.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: netdev@vger.kernel.org
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index aede704605c6..141c2a42d7ed 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -915,7 +915,7 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put(skb, IFLA_CAN_BITTIMING_CONST,
sizeof(*priv->bittiming_const), priv->bittiming_const)) ||
- nla_put(skb, IFLA_CAN_CLOCK, sizeof(cm), &priv->clock) ||
+ nla_put(skb, IFLA_CAN_CLOCK, sizeof(priv->clock), &priv->clock) ||
nla_put_u32(skb, IFLA_CAN_STATE, state) ||
nla_put(skb, IFLA_CAN_CTRLMODE, sizeof(cm), &cm) ||
nla_put_u32(skb, IFLA_CAN_RESTART_MS, priv->restart_ms) ||
--
2.6.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-30 15:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-30 12:48 [PATCH] can: Use correct type in sizeof() in nla_put() Marek Vasut
2015-10-30 13:40 ` Marc Kleine-Budde
2015-10-30 14:01 ` Marek Vasut
2015-10-30 14:17 ` Marc Kleine-Budde
2015-10-30 14:24 ` Marek Vasut
2015-10-30 14:53 ` Oliver Hartkopp
2015-10-30 15:33 ` Marek Vasut
2015-10-30 13:39 pull-request: can 2015-10-30 Marc Kleine-Budde
2015-10-30 13:39 ` [PATCH] can: Use correct type in sizeof() in nla_put() Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).