* [PATCH v2] can: bcm: check timer values before ktime conversion
@ 2019-01-13 18:31 Oliver Hartkopp
2019-01-16 13:17 ` Kyungtae Kim
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Oliver Hartkopp @ 2019-01-13 18:31 UTC (permalink / raw)
To: davem, netdev
Cc: linux-can, lifeasageek, threeearcat, syzkaller, nautsch2,
Oliver Hartkopp, Kyungtae Kim, linux-stable
Kyungtae Kim detected a potential integer overflow in bcm_[rx|tx]_setup()
when the conversion into ktime multiplies the given value with NSEC_PER_USEC
(1000).
Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
Add a check for the given tv_usec, so that the value stays below one second.
Additionally limit the tv_sec value to a reasonable value for CAN related
use-cases of 400 days and ensure all values to be positive.
Reported-by: Kyungtae Kim <kt0755@gmail.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org> # >= 2.6.26
---
net/can/bcm.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 0af8f0db892a..d4ae0a1471f3 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -67,6 +67,9 @@
*/
#define MAX_NFRAMES 256
+/* limit timers to 400 days for sending/timeouts */
+#define BCM_TIMER_SEC_MAX (400*24*60*60)
+
/* use of last_frames[index].flags */
#define RX_RECV 0x40 /* received data for this element */
#define RX_THR 0x80 /* element not been sent due to throttle feature */
@@ -140,6 +143,22 @@ static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv)
return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
}
+/* check limitations for timeval provided by user */
+static int bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
+{
+ if ((msg_head->ival1.tv_sec < 0) ||
+ (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
+ (msg_head->ival1.tv_usec < 0) ||
+ (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
+ (msg_head->ival2.tv_sec < 0) ||
+ (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
+ (msg_head->ival2.tv_usec < 0) ||
+ (msg_head->ival2.tv_usec >= USEC_PER_SEC))
+ return 1;
+
+ return 0;
+}
+
#define CFSIZ(flags) ((flags & CAN_FD_FRAME) ? CANFD_MTU : CAN_MTU)
#define OPSIZ sizeof(struct bcm_op)
#define MHSIZ sizeof(struct bcm_msg_head)
@@ -873,6 +892,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
return -EINVAL;
+ /* check timeval limitations */
+ if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
+ return -EINVAL;
+
/* check the given can_id */
op = bcm_find_op(&bo->tx_ops, msg_head, ifindex);
if (op) {
@@ -1053,6 +1076,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
(!(msg_head->can_id & CAN_RTR_FLAG))))
return -EINVAL;
+ /* check timeval limitations */
+ if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
+ return -EINVAL;
+
/* check the given can_id */
op = bcm_find_op(&bo->rx_ops, msg_head, ifindex);
if (op) {
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-13 18:31 [PATCH v2] can: bcm: check timer values before ktime conversion Oliver Hartkopp
@ 2019-01-16 13:17 ` Kyungtae Kim
2019-01-16 13:31 ` Andre Naujoks
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Kyungtae Kim @ 2019-01-16 13:17 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: davem, netdev, linux-can, Byoungyoung Lee, DaeRyong Jeong,
syzkaller, nautsch2, linux-stable
On Sun, Jan 13, 2019 at 1:31 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
> Kyungtae Kim detected a potential integer overflow in bcm_[rx|tx]_setup()
> when the conversion into ktime multiplies the given value with NSEC_PER_USEC
> (1000).
>
> Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
>
> Add a check for the given tv_usec, so that the value stays below one second.
> Additionally limit the tv_sec value to a reasonable value for CAN related
> use-cases of 400 days and ensure all values to be positive.
>
> Reported-by: Kyungtae Kim <kt0755@gmail.com>
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Tested-by: Kyungtae Kim <kt0755@gmail.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: linux-stable <stable@vger.kernel.org> # >= 2.6.26
> ---
> net/can/bcm.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 0af8f0db892a..d4ae0a1471f3 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -67,6 +67,9 @@
> */
> #define MAX_NFRAMES 256
>
> +/* limit timers to 400 days for sending/timeouts */
> +#define BCM_TIMER_SEC_MAX (400*24*60*60)
> +
> /* use of last_frames[index].flags */
> #define RX_RECV 0x40 /* received data for this element */
> #define RX_THR 0x80 /* element not been sent due to throttle feature */
> @@ -140,6 +143,22 @@ static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv)
> return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
> }
>
> +/* check limitations for timeval provided by user */
> +static int bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
> +{
> + if ((msg_head->ival1.tv_sec < 0) ||
> + (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
> + (msg_head->ival1.tv_usec < 0) ||
> + (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
> + (msg_head->ival2.tv_sec < 0) ||
> + (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
> + (msg_head->ival2.tv_usec < 0) ||
> + (msg_head->ival2.tv_usec >= USEC_PER_SEC))
> + return 1;
> +
> + return 0;
> +}
> +
> #define CFSIZ(flags) ((flags & CAN_FD_FRAME) ? CANFD_MTU : CAN_MTU)
> #define OPSIZ sizeof(struct bcm_op)
> #define MHSIZ sizeof(struct bcm_msg_head)
> @@ -873,6 +892,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
> return -EINVAL;
>
> + /* check timeval limitations */
> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
> + return -EINVAL;
> +
> /* check the given can_id */
> op = bcm_find_op(&bo->tx_ops, msg_head, ifindex);
> if (op) {
> @@ -1053,6 +1076,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> (!(msg_head->can_id & CAN_RTR_FLAG))))
> return -EINVAL;
>
> + /* check timeval limitations */
> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
> + return -EINVAL;
> +
> /* check the given can_id */
> op = bcm_find_op(&bo->rx_ops, msg_head, ifindex);
> if (op) {
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-13 18:31 [PATCH v2] can: bcm: check timer values before ktime conversion Oliver Hartkopp
2019-01-16 13:17 ` Kyungtae Kim
@ 2019-01-16 13:31 ` Andre Naujoks
2019-01-17 16:04 ` Oliver Hartkopp
[not found] ` <20190116133603.25A5A20873@mail.kernel.org>
2019-01-22 10:26 ` Marc Kleine-Budde
3 siblings, 1 reply; 9+ messages in thread
From: Andre Naujoks @ 2019-01-16 13:31 UTC (permalink / raw)
To: Oliver Hartkopp, davem, netdev
Cc: linux-can, lifeasageek, threeearcat, syzkaller, Kyungtae Kim,
linux-stable
Am 13.01.19 um 19:31 schrieb Oliver Hartkopp:
> Kyungtae Kim detected a potential integer overflow in bcm_[rx|tx]_setup()
> when the conversion into ktime multiplies the given value with NSEC_PER_USEC
> (1000).
>
> Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
>
> Add a check for the given tv_usec, so that the value stays below one second.
> Additionally limit the tv_sec value to a reasonable value for CAN related
> use-cases of 400 days and ensure all values to be positive.
>
> Reported-by: Kyungtae Kim <kt0755@gmail.com>
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: linux-stable <stable@vger.kernel.org> # >= 2.6.26
Acked-by: Andre Naujoks <nautsch2@gmail.com>
Sorry for the late reply, but I seem to have missed the initial send of
v2 of this. I wanted to at least ack it, since I made such a fuss about
the timeouts. :-)
Regards
Andre
> ---
> net/can/bcm.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 0af8f0db892a..d4ae0a1471f3 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -67,6 +67,9 @@
> */
> #define MAX_NFRAMES 256
>
> +/* limit timers to 400 days for sending/timeouts */
> +#define BCM_TIMER_SEC_MAX (400*24*60*60)
> +
> /* use of last_frames[index].flags */
> #define RX_RECV 0x40 /* received data for this element */
> #define RX_THR 0x80 /* element not been sent due to throttle feature */
> @@ -140,6 +143,22 @@ static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv)
> return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
> }
>
> +/* check limitations for timeval provided by user */
> +static int bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
> +{
> + if ((msg_head->ival1.tv_sec < 0) ||
> + (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
> + (msg_head->ival1.tv_usec < 0) ||
> + (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
> + (msg_head->ival2.tv_sec < 0) ||
> + (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
> + (msg_head->ival2.tv_usec < 0) ||
> + (msg_head->ival2.tv_usec >= USEC_PER_SEC))
> + return 1;
> +
> + return 0;
> +}
> +
> #define CFSIZ(flags) ((flags & CAN_FD_FRAME) ? CANFD_MTU : CAN_MTU)
> #define OPSIZ sizeof(struct bcm_op)
> #define MHSIZ sizeof(struct bcm_msg_head)
> @@ -873,6 +892,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
> return -EINVAL;
>
> + /* check timeval limitations */
> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
> + return -EINVAL;
> +
> /* check the given can_id */
> op = bcm_find_op(&bo->tx_ops, msg_head, ifindex);
> if (op) {
> @@ -1053,6 +1076,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> (!(msg_head->can_id & CAN_RTR_FLAG))))
> return -EINVAL;
>
> + /* check timeval limitations */
> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
> + return -EINVAL;
> +
> /* check the given can_id */
> op = bcm_find_op(&bo->rx_ops, msg_head, ifindex);
> if (op) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
[not found] ` <20190116133603.25A5A20873@mail.kernel.org>
@ 2019-01-16 15:05 ` Oliver Hartkopp
2019-01-16 15:54 ` Oliver Hartkopp
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2019-01-16 15:05 UTC (permalink / raw)
To: Sasha Levin, davem, netdev; +Cc: linux-can, lifeasageek, stable
Hi Sasha,
On 1/16/19 2:36 PM, Sasha Levin wrote:
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: 2.6.26+
>
> The bot has tested the following trees: v4.20.2, v4.19.15, v4.14.93, v4.9.150, v4.4.170, v3.18.132.
>
> v4.20.2: Build OK!
> v4.19.15: Build OK!
> v4.14.93: Build OK!
> v4.9.150: Build OK!
> v4.4.170: Failed to apply! Possible dependencies:
> 2b5f5f5dc114 ("can: bcm: unify bcm_msg_head handling and prepare function parameters")
> 6f3b911d5f29 ("can: bcm: add support for CAN FD frames")
> 72c8a89ad2e4 ("can: bcm: use CAN frame instead of can_frame in comments")
> 95acb490ec51 ("can: bcm: fix indention and other minor style issues")
>
> v3.18.132: Failed to apply! Possible dependencies:
> 069f8457ae52 ("can: fix spelling errors")
> 2b5f5f5dc114 ("can: bcm: unify bcm_msg_head handling and prepare function parameters")
> 6ce8e9ce5989 ("new helper: memcpy_from_msg()")
> 6f3b911d5f29 ("can: bcm: add support for CAN FD frames")
> 72c8a89ad2e4 ("can: bcm: use CAN frame instead of can_frame in comments")
> 95acb490ec51 ("can: bcm: fix indention and other minor style issues")
> ba61a8d9d780 ("can: avoid using timeval for uapi")
>
>
> How should we proceed with this patch?
Applying the patch on e.g. 3.2.102 also leads to
patching file net/can/bcm.c
Hunk #1 FAILED at 67.
Hunk #2 FAILED at 140.
Hunk #3 succeeded at 847 with fuzz 2 (offset -26 lines).
Hunk #4 succeeded at 1018 with fuzz 2 (offset -39 lines).
2 out of 4 hunks FAILED -- saving rejects to file net/can/bcm.c.rej
The first two hunks just adding a define and and function *somewhere* at
the top of the C file.
I can provide patches for the requested stable kernels once we have a
reference for the upstream commit hash.
Would that be ok for you?
Best regards,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-16 15:05 ` Oliver Hartkopp
@ 2019-01-16 15:54 ` Oliver Hartkopp
0 siblings, 0 replies; 9+ messages in thread
From: Oliver Hartkopp @ 2019-01-16 15:54 UTC (permalink / raw)
To: Sasha Levin, davem, netdev; +Cc: linux-can, lifeasageek, stable
Hi Sasha,
some more details ...
> On 1/16/19 2:36 PM, Sasha Levin wrote:
>> This commit has been processed because it contains a -stable tag.
>> The stable tag indicates that it's relevant for the following trees:
>> 2.6.26+
>>
>> v4.9.150: Build OK!
>> v4.4.170: Failed to apply! Possible dependencies:
This patch introduced in 4.8 leads to the problem:
>> 6f3b911d5f29 ("can: bcm: add support for CAN FD frames")
I will provide a patch which applies to all Linux pre-4.8 kernels once
we have an upstream commit hash.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-16 13:31 ` Andre Naujoks
@ 2019-01-17 16:04 ` Oliver Hartkopp
2019-01-22 10:16 ` Oliver Hartkopp
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2019-01-17 16:04 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Andre Naujoks, davem, netdev, linux-can, lifeasageek,
threeearcat, syzkaller, Kyungtae Kim, linux-stable
Hello Marc,
I would like to provide a stable patch for pre-4.18 kernels and need a
commit hash as reference.
Would you like to take that patch into upstream?
Best regards,
Oliver
On 1/16/19 2:31 PM, Andre Naujoks wrote:
> Am 13.01.19 um 19:31 schrieb Oliver Hartkopp:
>> Kyungtae Kim detected a potential integer overflow in bcm_[rx|tx]_setup()
>> when the conversion into ktime multiplies the given value with NSEC_PER_USEC
>> (1000).
>>
>> Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
>>
>> Add a check for the given tv_usec, so that the value stays below one second.
>> Additionally limit the tv_sec value to a reasonable value for CAN related
>> use-cases of 400 days and ensure all values to be positive.
>>
>> Reported-by: Kyungtae Kim <kt0755@gmail.com>
>> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> Cc: linux-stable <stable@vger.kernel.org> # >= 2.6.26
>
> Acked-by: Andre Naujoks <nautsch2@gmail.com>
>
> Sorry for the late reply, but I seem to have missed the initial send of
> v2 of this. I wanted to at least ack it, since I made such a fuss about
> the timeouts. :-)
>
> Regards
> Andre
>
>> ---
>> net/can/bcm.c | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>> index 0af8f0db892a..d4ae0a1471f3 100644
>> --- a/net/can/bcm.c
>> +++ b/net/can/bcm.c
>> @@ -67,6 +67,9 @@
>> */
>> #define MAX_NFRAMES 256
>>
>> +/* limit timers to 400 days for sending/timeouts */
>> +#define BCM_TIMER_SEC_MAX (400*24*60*60)
>> +
>> /* use of last_frames[index].flags */
>> #define RX_RECV 0x40 /* received data for this element */
>> #define RX_THR 0x80 /* element not been sent due to throttle feature */
>> @@ -140,6 +143,22 @@ static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv)
>> return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
>> }
>>
>> +/* check limitations for timeval provided by user */
>> +static int bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
>> +{
>> + if ((msg_head->ival1.tv_sec < 0) ||
>> + (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
>> + (msg_head->ival1.tv_usec < 0) ||
>> + (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
>> + (msg_head->ival2.tv_sec < 0) ||
>> + (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
>> + (msg_head->ival2.tv_usec < 0) ||
>> + (msg_head->ival2.tv_usec >= USEC_PER_SEC))
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>> #define CFSIZ(flags) ((flags & CAN_FD_FRAME) ? CANFD_MTU : CAN_MTU)
>> #define OPSIZ sizeof(struct bcm_op)
>> #define MHSIZ sizeof(struct bcm_msg_head)
>> @@ -873,6 +892,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>> if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
>> return -EINVAL;
>>
>> + /* check timeval limitations */
>> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
>> + return -EINVAL;
>> +
>> /* check the given can_id */
>> op = bcm_find_op(&bo->tx_ops, msg_head, ifindex);
>> if (op) {
>> @@ -1053,6 +1076,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>> (!(msg_head->can_id & CAN_RTR_FLAG))))
>> return -EINVAL;
>>
>> + /* check timeval limitations */
>> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
>> + return -EINVAL;
>> +
>> /* check the given can_id */
>> op = bcm_find_op(&bo->rx_ops, msg_head, ifindex);
>> if (op) {
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-17 16:04 ` Oliver Hartkopp
@ 2019-01-22 10:16 ` Oliver Hartkopp
2019-01-22 10:19 ` Marc Kleine-Budde
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2019-01-22 10:16 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Kyungtae Kim, linux-stable
Ping?
If you have no time I can just ask Dave. But please drop me a short note.
Thanks,
Oliver
On 17.01.19 17:04, Oliver Hartkopp wrote:
> Hello Marc,
>
> I would like to provide a stable patch for pre-4.18 kernels and need a
> commit hash as reference.
>
> Would you like to take that patch into upstream?
>
> Best regards,
> Oliver
>
> On 1/16/19 2:31 PM, Andre Naujoks wrote:
>> Am 13.01.19 um 19:31 schrieb Oliver Hartkopp:
>>> Kyungtae Kim detected a potential integer overflow in
>>> bcm_[rx|tx]_setup()
>>> when the conversion into ktime multiplies the given value with
>>> NSEC_PER_USEC
>>> (1000).
>>>
>>> Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
>>>
>>> Add a check for the given tv_usec, so that the value stays below one
>>> second.
>>> Additionally limit the tv_sec value to a reasonable value for CAN
>>> related
>>> use-cases of 400 days and ensure all values to be positive.
>>>
>>> Reported-by: Kyungtae Kim <kt0755@gmail.com>
>>> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>> Cc: linux-stable <stable@vger.kernel.org> # >= 2.6.26
>>
>> Acked-by: Andre Naujoks <nautsch2@gmail.com>
>>
>> Sorry for the late reply, but I seem to have missed the initial send of
>> v2 of this. I wanted to at least ack it, since I made such a fuss about
>> the timeouts. :-)
>>
>> Regards
>> Andre
>>
>>> ---
>>> net/can/bcm.c | 27 +++++++++++++++++++++++++++
>>> 1 file changed, 27 insertions(+)
>>>
>>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>>> index 0af8f0db892a..d4ae0a1471f3 100644
>>> --- a/net/can/bcm.c
>>> +++ b/net/can/bcm.c
>>> @@ -67,6 +67,9 @@
>>> */
>>> #define MAX_NFRAMES 256
>>> +/* limit timers to 400 days for sending/timeouts */
>>> +#define BCM_TIMER_SEC_MAX (400*24*60*60)
>>> +
>>> /* use of last_frames[index].flags */
>>> #define RX_RECV 0x40 /* received data for this element */
>>> #define RX_THR 0x80 /* element not been sent due to throttle
>>> feature */
>>> @@ -140,6 +143,22 @@ static inline ktime_t
>>> bcm_timeval_to_ktime(struct bcm_timeval tv)
>>> return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
>>> }
>>> +/* check limitations for timeval provided by user */
>>> +static int bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
>>> +{
>>> + if ((msg_head->ival1.tv_sec < 0) ||
>>> + (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
>>> + (msg_head->ival1.tv_usec < 0) ||
>>> + (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
>>> + (msg_head->ival2.tv_sec < 0) ||
>>> + (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
>>> + (msg_head->ival2.tv_usec < 0) ||
>>> + (msg_head->ival2.tv_usec >= USEC_PER_SEC))
>>> + return 1;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> #define CFSIZ(flags) ((flags & CAN_FD_FRAME) ? CANFD_MTU : CAN_MTU)
>>> #define OPSIZ sizeof(struct bcm_op)
>>> #define MHSIZ sizeof(struct bcm_msg_head)
>>> @@ -873,6 +892,10 @@ static int bcm_tx_setup(struct bcm_msg_head
>>> *msg_head, struct msghdr *msg,
>>> if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
>>> return -EINVAL;
>>> + /* check timeval limitations */
>>> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
>>> + return -EINVAL;
>>> +
>>> /* check the given can_id */
>>> op = bcm_find_op(&bo->tx_ops, msg_head, ifindex);
>>> if (op) {
>>> @@ -1053,6 +1076,10 @@ static int bcm_rx_setup(struct bcm_msg_head
>>> *msg_head, struct msghdr *msg,
>>> (!(msg_head->can_id & CAN_RTR_FLAG))))
>>> return -EINVAL;
>>> + /* check timeval limitations */
>>> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
>>> + return -EINVAL;
>>> +
>>> /* check the given can_id */
>>> op = bcm_find_op(&bo->rx_ops, msg_head, ifindex);
>>> if (op) {
>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-22 10:16 ` Oliver Hartkopp
@ 2019-01-22 10:19 ` Marc Kleine-Budde
0 siblings, 0 replies; 9+ messages in thread
From: Marc Kleine-Budde @ 2019-01-22 10:19 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Kyungtae Kim, linux-stable
[-- Attachment #1.1: Type: text/plain, Size: 452 bytes --]
On 1/22/19 11:16 AM, Oliver Hartkopp wrote:
> Ping?
> If you have no time I can just ask Dave. But please drop me a short note.
Pong. I'll take the patch now.
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: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] can: bcm: check timer values before ktime conversion
2019-01-13 18:31 [PATCH v2] can: bcm: check timer values before ktime conversion Oliver Hartkopp
` (2 preceding siblings ...)
[not found] ` <20190116133603.25A5A20873@mail.kernel.org>
@ 2019-01-22 10:26 ` Marc Kleine-Budde
3 siblings, 0 replies; 9+ messages in thread
From: Marc Kleine-Budde @ 2019-01-22 10:26 UTC (permalink / raw)
To: Oliver Hartkopp, davem, netdev
Cc: linux-can, lifeasageek, threeearcat, syzkaller, nautsch2,
Kyungtae Kim, linux-stable
[-- Attachment #1.1: Type: text/plain, Size: 3479 bytes --]
On 1/13/19 7:31 PM, Oliver Hartkopp wrote:
> Kyungtae Kim detected a potential integer overflow in bcm_[rx|tx]_setup()
> when the conversion into ktime multiplies the given value with NSEC_PER_USEC
> (1000).
>
> Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
>
> Add a check for the given tv_usec, so that the value stays below one second.
> Additionally limit the tv_sec value to a reasonable value for CAN related
> use-cases of 400 days and ensure all values to be positive.
>
> Reported-by: Kyungtae Kim <kt0755@gmail.com>
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: linux-stable <stable@vger.kernel.org> # >= 2.6.26
> ---
> net/can/bcm.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 0af8f0db892a..d4ae0a1471f3 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -67,6 +67,9 @@
> */
> #define MAX_NFRAMES 256
>
> +/* limit timers to 400 days for sending/timeouts */
> +#define BCM_TIMER_SEC_MAX (400*24*60*60)
I've added spaces around the * while applying the patch.
> +
> /* use of last_frames[index].flags */
> #define RX_RECV 0x40 /* received data for this element */
> #define RX_THR 0x80 /* element not been sent due to throttle feature */
> @@ -140,6 +143,22 @@ static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv)
> return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
> }
>
> +/* check limitations for timeval provided by user */
> +static int bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
I've converted this into a bool function.
> +{
> + if ((msg_head->ival1.tv_sec < 0) ||
> + (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
> + (msg_head->ival1.tv_usec < 0) ||
> + (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
> + (msg_head->ival2.tv_sec < 0) ||
> + (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
> + (msg_head->ival2.tv_usec < 0) ||
> + (msg_head->ival2.tv_usec >= USEC_PER_SEC))
> + return 1;
return true;
> +
> + return 0;
return false;
> +}
> +
> #define CFSIZ(flags) ((flags & CAN_FD_FRAME) ? CANFD_MTU : CAN_MTU)
> #define OPSIZ sizeof(struct bcm_op)
> #define MHSIZ sizeof(struct bcm_msg_head)
> @@ -873,6 +892,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
> return -EINVAL;
>
> + /* check timeval limitations */
> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
> + return -EINVAL;
> +
> /* check the given can_id */
> op = bcm_find_op(&bo->tx_ops, msg_head, ifindex);
> if (op) {
> @@ -1053,6 +1076,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> (!(msg_head->can_id & CAN_RTR_FLAG))))
> return -EINVAL;
>
> + /* check timeval limitations */
> + if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
> + return -EINVAL;
> +
> /* check the given can_id */
> op = bcm_find_op(&bo->rx_ops, msg_head, ifindex);
> if (op) {
>
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: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-22 10:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-13 18:31 [PATCH v2] can: bcm: check timer values before ktime conversion Oliver Hartkopp
2019-01-16 13:17 ` Kyungtae Kim
2019-01-16 13:31 ` Andre Naujoks
2019-01-17 16:04 ` Oliver Hartkopp
2019-01-22 10:16 ` Oliver Hartkopp
2019-01-22 10:19 ` Marc Kleine-Budde
[not found] ` <20190116133603.25A5A20873@mail.kernel.org>
2019-01-16 15:05 ` Oliver Hartkopp
2019-01-16 15:54 ` Oliver Hartkopp
2019-01-22 10:26 ` 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).