Message ID | 1324493459-19764-1-git-send-email-xi.wang@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Le mercredi 21 décembre 2011 à 13:50 -0500, Xi Wang a écrit : > Setting a large rps_flow_cnt like 1073741824 (1 << 30) on 32-bit > platform will cause a kernel oops due to insufficient bounds checking. > > if (count > 1<<30) { > /* Enforce a limit to prevent overflow */ > return -EINVAL; > } > count = roundup_pow_of_two(count); > table = vmalloc(RPS_DEV_FLOW_TABLE_SIZE(count)); > > Note that the macro RPS_DEV_FLOW_TABLE_SIZE(count) is defined as: > > ... + (count * sizeof(struct rps_dev_flow)) > > where sizeof(struct rps_dev_flow) is 8. (1 << 30) * 8 will overflow > 32 bits. This patch changes the upper bound to (1 << 28). > > Signed-off-by: Xi Wang <xi.wang@gmail.com> > --- > net/core/net-sysfs.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index c71c434..f53a947 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -665,7 +665,7 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue, > if (count) { > int i; > > - if (count > 1<<30) { > + if (count > 1<<28) { > /* Enforce a limit to prevent overflow */ > return -EINVAL; > } Really, you should remove this magic number and use instead (INT_MAX - RPS_DEV_FLOW_TABLE_SIZE(0)) / sizeof(struct rps_dev_flow) Or something like that, because next time we add a field in rps_dev_flow, test will be obsolete. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 21 Dec 2011 20:22:24 +0100 > Le mercredi 21 décembre 2011 à 13:50 -0500, Xi Wang a écrit : >> @@ -665,7 +665,7 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue, >> if (count) { >> int i; >> >> - if (count > 1<<30) { >> + if (count > 1<<28) { >> /* Enforce a limit to prevent overflow */ >> return -EINVAL; >> } > > > Really, you should remove this magic number and use instead > > (INT_MAX - RPS_DEV_FLOW_TABLE_SIZE(0)) / sizeof(struct rps_dev_flow) > > Or something like that, because next time we add a field in > rps_dev_flow, test will be obsolete. Agreed. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Agreed too. ;-) Thanks for the suggestion. I will do a v2 without using the magic number. BTW, a similar magic number (1<<30) is used in rps_sock_flow_sysctl() at net/core/sysctl_net_core.c. I will change that as well. - xi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index c71c434..f53a947 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -665,7 +665,7 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue, if (count) { int i; - if (count > 1<<30) { + if (count > 1<<28) { /* Enforce a limit to prevent overflow */ return -EINVAL; }
Setting a large rps_flow_cnt like 1073741824 (1 << 30) on 32-bit platform will cause a kernel oops due to insufficient bounds checking. if (count > 1<<30) { /* Enforce a limit to prevent overflow */ return -EINVAL; } count = roundup_pow_of_two(count); table = vmalloc(RPS_DEV_FLOW_TABLE_SIZE(count)); Note that the macro RPS_DEV_FLOW_TABLE_SIZE(count) is defined as: ... + (count * sizeof(struct rps_dev_flow)) where sizeof(struct rps_dev_flow) is 8. (1 << 30) * 8 will overflow 32 bits. This patch changes the upper bound to (1 << 28). Signed-off-by: Xi Wang <xi.wang@gmail.com> --- net/core/net-sysfs.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)