diff mbox series

[stable,pre-4.8] can: bcm: check timer values before ktime conversion

Message ID 20190124090842.2938-1-socketcan@hartkopp.net
State Not Applicable
Delegated to: David Miller
Headers show
Series [stable,pre-4.8] can: bcm: check timer values before ktime conversion | expand

Commit Message

Oliver Hartkopp Jan. 24, 2019, 9:08 a.m. UTC 
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.

This patch is the pre-4.8 version of upstream commit 93171ba6f1deffd8

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> # versions 2.6.26 to 4.7
Tested-by: Kyungtae Kim <kt0755@gmail.com>
Acked-by: Andre Naujoks <nautsch2@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/bcm.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Sasha Levin Jan. 26, 2019, 6:17 p.m. UTC | #1 
On Thu, Jan 24, 2019 at 10:08:42AM +0100, 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.
>
>This patch is the pre-4.8 version of upstream commit 93171ba6f1deffd8

I can't find this commit id upstream, there's nothing with the same
subject name, nor does this code exist upstream. What's going on?

--
Thanks,
Sasha
Oliver Hartkopp Jan. 27, 2019, 6:22 p.m. UTC | #2 
Hi Sasha,

On 26.01.19 19:17, Sasha Levin wrote:
> On Thu, Jan 24, 2019 at 10:08:42AM +0100, 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.
>>
>> This patch is the pre-4.8 version of upstream commit 93171ba6f1deffd8
> 
> I can't find this commit id upstream, there's nothing with the same
> subject name, nor does this code exist upstream. What's going on?

Here we are (pulled by Linus some minutes ago):

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93171ba6f1deffd82f381d36cb13177872d023f6

Can you go with this pre-4.8 version now?

Many thanks,
Oliver
Greg KH Jan. 27, 2019, 7:34 p.m. UTC | #3 
On Sun, Jan 27, 2019 at 07:22:38PM +0100, Oliver Hartkopp wrote:
> Hi Sasha,
> 
> On 26.01.19 19:17, Sasha Levin wrote:
> > On Thu, Jan 24, 2019 at 10:08:42AM +0100, 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.
> > > 
> > > This patch is the pre-4.8 version of upstream commit 93171ba6f1deffd8
> > 
> > I can't find this commit id upstream, there's nothing with the same
> > subject name, nor does this code exist upstream. What's going on?
> 
> Here we are (pulled by Linus some minutes ago):
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93171ba6f1deffd82f381d36cb13177872d023f6
> 
> Can you go with this pre-4.8 version now?

Let me get the others into the newer kernels first please :)
Should be a day or so at most...

thanks,

greg k-h
diff mbox series

Patch

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 6863310d6973..01d489d0a3de 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].can_dlc */
 #define RX_RECV    0x40 /* received data for this element */
 #define RX_THR     0x80 /* element not been sent due to throttle feature */
@@ -136,6 +139,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 bool 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 true;
+
+	return false;
+}
+
 #define CFSIZ sizeof(struct can_frame)
 #define OPSIZ sizeof(struct bcm_op)
 #define MHSIZ sizeof(struct bcm_msg_head)
@@ -846,6 +865,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->can_id, ifindex);
 
@@ -1011,6 +1034,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->can_id, ifindex);
 	if (op) {