* [PATCH] rbd: remove VLA usage
@ 2018-03-30 19:17 Gustavo A. R. Silva
2018-03-30 20:29 ` Ilya Dryomov
0 siblings, 1 reply; 7+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-30 19:17 UTC (permalink / raw)
To: Ilya Dryomov, Sage Weil, Alex Elder, Jens Axboe
Cc: ceph-devel, linux-block, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wvla, remove the use of stack VLA.
In this particular case, variable buf_size is replaced with a constant
expression that can be computed at preprocessing time. This avoids two
VLA warnings. Also, as a consequence of the mentioned change, some code
was slightly refactored.
The use of stack Variable Length Arrays needs to be avoided, as they
can be a vector for stack exhaustion, which can be both a runtime bug
or a security flaw. Also, in general, as code evolves it is easy to
lose track of how big a VLA can get. Thus, we can end up having runtime
failures that are hard to debug.
Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/block/rbd.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 1e03b04..5133122 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3091,20 +3091,20 @@ static int __rbd_notify_op_lock(struct rbd_device *rbd_dev,
{
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
struct rbd_client_id cid = rbd_get_cid(rbd_dev);
- int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
- char buf[buf_size];
+ char buf[4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
void *p = buf;
dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
/* encode *LockPayload NotifyMessage (op + ClientId) */
- ceph_start_encoding(&p, 2, 1, buf_size - CEPH_ENCODING_START_BLK_LEN);
+ ceph_start_encoding(&p, 2, 1,
+ sizeof(buf) - CEPH_ENCODING_START_BLK_LEN);
ceph_encode_32(&p, notify_op);
ceph_encode_64(&p, cid.gid);
ceph_encode_64(&p, cid.handle);
return ceph_osdc_notify(osdc, &rbd_dev->header_oid,
- &rbd_dev->header_oloc, buf, buf_size,
+ &rbd_dev->header_oloc, buf, sizeof(buf),
RBD_NOTIFY_TIMEOUT, preply_pages, preply_len);
}
@@ -3610,19 +3610,18 @@ static void __rbd_acknowledge_notify(struct rbd_device *rbd_dev,
u64 notify_id, u64 cookie, s32 *result)
{
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
- int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
- char buf[buf_size];
+ char buf[4 + CEPH_ENCODING_START_BLK_LEN];
+ int buf_size = 0;
int ret;
if (result) {
void *p = buf;
+ buf_size = sizeof(buf);
/* encode ResponseMessage */
ceph_start_encoding(&p, 1, 1,
buf_size - CEPH_ENCODING_START_BLK_LEN);
ceph_encode_32(&p, *result);
- } else {
- buf_size = 0;
}
ret = ceph_osdc_notify_ack(osdc, &rbd_dev->header_oid,
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] rbd: remove VLA usage
2018-03-30 19:17 [PATCH] rbd: remove VLA usage Gustavo A. R. Silva
@ 2018-03-30 20:29 ` Ilya Dryomov
2018-03-30 20:34 ` Gustavo A. R. Silva
0 siblings, 1 reply; 7+ messages in thread
From: Ilya Dryomov @ 2018-03-30 20:29 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Sage Weil, Alex Elder, Jens Axboe, Ceph Development, linux-block,
linux-kernel
On Fri, Mar 30, 2018 at 9:17 PM, Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wvla, remove the use of stack VLA.
>
> In this particular case, variable buf_size is replaced with a constant
> expression that can be computed at preprocessing time. This avoids two
> VLA warnings. Also, as a consequence of the mentioned change, some code
> was slightly refactored.
>
> The use of stack Variable Length Arrays needs to be avoided, as they
> can be a vector for stack exhaustion, which can be both a runtime bug
> or a security flaw. Also, in general, as code evolves it is easy to
> lose track of how big a VLA can get. Thus, we can end up having runtime
> failures that are hard to debug.
>
> Also, fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/block/rbd.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 1e03b04..5133122 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3091,20 +3091,20 @@ static int __rbd_notify_op_lock(struct rbd_device *rbd_dev,
> {
> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> struct rbd_client_id cid = rbd_get_cid(rbd_dev);
> - int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
> - char buf[buf_size];
> + char buf[4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
> void *p = buf;
>
> dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
>
> /* encode *LockPayload NotifyMessage (op + ClientId) */
> - ceph_start_encoding(&p, 2, 1, buf_size - CEPH_ENCODING_START_BLK_LEN);
> + ceph_start_encoding(&p, 2, 1,
> + sizeof(buf) - CEPH_ENCODING_START_BLK_LEN);
> ceph_encode_32(&p, notify_op);
> ceph_encode_64(&p, cid.gid);
> ceph_encode_64(&p, cid.handle);
>
> return ceph_osdc_notify(osdc, &rbd_dev->header_oid,
> - &rbd_dev->header_oloc, buf, buf_size,
> + &rbd_dev->header_oloc, buf, sizeof(buf),
> RBD_NOTIFY_TIMEOUT, preply_pages, preply_len);
> }
>
> @@ -3610,19 +3610,18 @@ static void __rbd_acknowledge_notify(struct rbd_device *rbd_dev,
> u64 notify_id, u64 cookie, s32 *result)
> {
> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> - int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
> - char buf[buf_size];
> + char buf[4 + CEPH_ENCODING_START_BLK_LEN];
> + int buf_size = 0;
> int ret;
>
> if (result) {
> void *p = buf;
>
> + buf_size = sizeof(buf);
> /* encode ResponseMessage */
> ceph_start_encoding(&p, 1, 1,
> buf_size - CEPH_ENCODING_START_BLK_LEN);
> ceph_encode_32(&p, *result);
> - } else {
> - buf_size = 0;
> }
>
> ret = ceph_osdc_notify_ack(osdc, &rbd_dev->header_oid,
Already fixed:
https://github.com/ceph/ceph-client/commit/2474e77e75215a3cad3b652d2f55ba5b123cb119
Thanks,
Ilya
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rbd: remove VLA usage
2018-03-30 20:29 ` Ilya Dryomov
@ 2018-03-30 20:34 ` Gustavo A. R. Silva
0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-30 20:34 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Sage Weil, Alex Elder, Jens Axboe, Ceph Development, linux-block,
linux-kernel
On 03/30/2018 03:29 PM, Ilya Dryomov wrote:
> On Fri, Mar 30, 2018 at 9:17 PM, Gustavo A. R. Silva
> <gustavo@embeddedor.com> wrote:
>> In preparation to enabling -Wvla, remove the use of stack VLA.
>>
>> In this particular case, variable buf_size is replaced with a constant
>> expression that can be computed at preprocessing time. This avoids two
>> VLA warnings. Also, as a consequence of the mentioned change, some code
>> was slightly refactored.
>>
>> The use of stack Variable Length Arrays needs to be avoided, as they
>> can be a vector for stack exhaustion, which can be both a runtime bug
>> or a security flaw. Also, in general, as code evolves it is easy to
>> lose track of how big a VLA can get. Thus, we can end up having runtime
>> failures that are hard to debug.
>>
>> Also, fixed as part of the directive to remove all VLAs from
>> the kernel: https://lkml.org/lkml/2018/3/7/621
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> drivers/block/rbd.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 1e03b04..5133122 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -3091,20 +3091,20 @@ static int __rbd_notify_op_lock(struct rbd_device *rbd_dev,
>> {
>> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
>> struct rbd_client_id cid = rbd_get_cid(rbd_dev);
>> - int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
>> - char buf[buf_size];
>> + char buf[4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
>> void *p = buf;
>>
>> dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
>>
>> /* encode *LockPayload NotifyMessage (op + ClientId) */
>> - ceph_start_encoding(&p, 2, 1, buf_size - CEPH_ENCODING_START_BLK_LEN);
>> + ceph_start_encoding(&p, 2, 1,
>> + sizeof(buf) - CEPH_ENCODING_START_BLK_LEN);
>> ceph_encode_32(&p, notify_op);
>> ceph_encode_64(&p, cid.gid);
>> ceph_encode_64(&p, cid.handle);
>>
>> return ceph_osdc_notify(osdc, &rbd_dev->header_oid,
>> - &rbd_dev->header_oloc, buf, buf_size,
>> + &rbd_dev->header_oloc, buf, sizeof(buf),
>> RBD_NOTIFY_TIMEOUT, preply_pages, preply_len);
>> }
>>
>> @@ -3610,19 +3610,18 @@ static void __rbd_acknowledge_notify(struct rbd_device *rbd_dev,
>> u64 notify_id, u64 cookie, s32 *result)
>> {
>> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
>> - int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
>> - char buf[buf_size];
>> + char buf[4 + CEPH_ENCODING_START_BLK_LEN];
>> + int buf_size = 0;
>> int ret;
>>
>> if (result) {
>> void *p = buf;
>>
>> + buf_size = sizeof(buf);
>> /* encode ResponseMessage */
>> ceph_start_encoding(&p, 1, 1,
>> buf_size - CEPH_ENCODING_START_BLK_LEN);
>> ceph_encode_32(&p, *result);
>> - } else {
>> - buf_size = 0;
>> }
>>
>> ret = ceph_osdc_notify_ack(osdc, &rbd_dev->header_oid,
>
> Already fixed:
>
> https://github.com/ceph/ceph-client/commit/2474e77e75215a3cad3b652d2f55ba5b123cb119
>
Oh, that's great.
Good to know. :)
Thanks
--
Gustavo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rbd: Remove VLA usage
2018-03-10 5:14 ` Kyle Spiers
(?)
(?)
@ 2018-03-12 9:45 ` Ilya Dryomov
-1 siblings, 0 replies; 7+ messages in thread
From: Ilya Dryomov @ 2018-03-12 9:45 UTC (permalink / raw)
To: Kyle Spiers
Cc: Sage Weil, Alex Elder, Ceph Development, linux-kernel, Kees Cook
On Sat, Mar 10, 2018 at 6:14 AM, Kyle Spiers <kyle@spiers.me> wrote:
> From 4198ebe2e8058ff676d8e2f993d8806d6ca29c11 Mon Sep 17 00:00:00 2001
> From: Kyle Spiers <kyle@spiers.me>
> Date: Fri, 9 Mar 2018 12:34:15 -0800
> Subject: [PATCH] rbd: Remove VLA usage
>
> As part of the effort to remove VLAs from the kernel[1], this moves
> the literal values into the stack array calculation instead of using a
> variable for the sizing. The resulting size can be found from
> sizeof(buf).
>
> [1] https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Kyle Spiers <kyle@spiers.me>
>
> ---
> drivers/block/rbd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 8e40da0..0e94e1f 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3100,8 +3100,8 @@ static int __rbd_notify_op_lock(struct rbd_device
> *rbd_dev,
> {
> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> struct rbd_client_id cid = rbd_get_cid(rbd_dev);
> - int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
> - char buf[buf_size];
> + char buf[4 + 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
> + int buf_size = sizeof(buf);
> void *p = buf;
>
> dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
> @@ -3619,8 +3619,8 @@ static void __rbd_acknowledge_notify(struct
> rbd_device *rbd_dev,
> u64 notify_id, u64 cookie, s32 *result)
> {
> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> - int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
> - char buf[buf_size];
> + char buf[4 + CEPH_ENCODING_START_BLK_LEN];
> + int buf_size = sizeof(buf);
> int ret;
>
> if (result) {
This patch is mangled -- tabs, etc. See
https://www.kernel.org/doc/html/v4.15/process/email-clients.html
Thanks,
Ilya
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rbd: Remove VLA usage
2018-03-10 5:14 ` Kyle Spiers
(?)
@ 2018-03-10 15:24 ` Kees Cook
-1 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2018-03-10 15:24 UTC (permalink / raw)
To: Kyle Spiers; +Cc: Ilya Dryomov, Sage Weil, Alex Elder, ceph-devel, LKML
On Fri, Mar 9, 2018 at 9:14 PM, Kyle Spiers <kyle@spiers.me> wrote:
> From 4198ebe2e8058ff676d8e2f993d8806d6ca29c11 Mon Sep 17 00:00:00 2001
> From: Kyle Spiers <kyle@spiers.me>
> Date: Fri, 9 Mar 2018 12:34:15 -0800
> Subject: [PATCH] rbd: Remove VLA usage
>
> As part of the effort to remove VLAs from the kernel[1], this moves
> the literal values into the stack array calculation instead of using a
> variable for the sizing. The resulting size can be found from
> sizeof(buf).
>
> [1] https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Kyle Spiers <kyle@spiers.me>
Thanks!
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
>
> ---
> drivers/block/rbd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 8e40da0..0e94e1f 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3100,8 +3100,8 @@ static int __rbd_notify_op_lock(struct rbd_device
> *rbd_dev,
> {
> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> struct rbd_client_id cid = rbd_get_cid(rbd_dev);
> - int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
> - char buf[buf_size];
> + char buf[4 + 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
> + int buf_size = sizeof(buf);
> void *p = buf;
>
> dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
> @@ -3619,8 +3619,8 @@ static void __rbd_acknowledge_notify(struct
> rbd_device *rbd_dev,
> u64 notify_id, u64 cookie, s32 *result)
> {
> struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
> - int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
> - char buf[buf_size];
> + char buf[4 + CEPH_ENCODING_START_BLK_LEN];
> + int buf_size = sizeof(buf);
> int ret;
>
> if (result) {
> -- 2.7.4
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] rbd: Remove VLA usage
@ 2018-03-10 5:14 ` Kyle Spiers
0 siblings, 0 replies; 7+ messages in thread
From: Kyle Spiers @ 2018-03-10 5:14 UTC (permalink / raw)
To: idryomov; +Cc: sage, elder, ceph-devel, linux-kernel, keescook
>From 4198ebe2e8058ff676d8e2f993d8806d6ca29c11 Mon Sep 17 00:00:00 2001
From: Kyle Spiers <kyle@spiers.me>
Date: Fri, 9 Mar 2018 12:34:15 -0800
Subject: [PATCH] rbd: Remove VLA usage
As part of the effort to remove VLAs from the kernel[1], this moves
the literal values into the stack array calculation instead of using a
variable for the sizing. The resulting size can be found from
sizeof(buf).
[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by: Kyle Spiers <kyle@spiers.me>
---
drivers/block/rbd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 8e40da0..0e94e1f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3100,8 +3100,8 @@ static int __rbd_notify_op_lock(struct rbd_device
*rbd_dev,
{
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
struct rbd_client_id cid = rbd_get_cid(rbd_dev);
- int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
- char buf[buf_size];
+ char buf[4 + 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
+ int buf_size = sizeof(buf);
void *p = buf;
dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
@@ -3619,8 +3619,8 @@ static void __rbd_acknowledge_notify(struct
rbd_device *rbd_dev,
u64 notify_id, u64 cookie, s32 *result)
{
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
- int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
- char buf[buf_size];
+ char buf[4 + CEPH_ENCODING_START_BLK_LEN];
+ int buf_size = sizeof(buf);
int ret;
if (result) {
-- 2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] rbd: Remove VLA usage
@ 2018-03-10 5:14 ` Kyle Spiers
0 siblings, 0 replies; 7+ messages in thread
From: Kyle Spiers @ 2018-03-10 5:14 UTC (permalink / raw)
To: idryomov; +Cc: sage, elder, ceph-devel, linux-kernel, keescook
From 4198ebe2e8058ff676d8e2f993d8806d6ca29c11 Mon Sep 17 00:00:00 2001
From: Kyle Spiers <kyle@spiers.me>
Date: Fri, 9 Mar 2018 12:34:15 -0800
Subject: [PATCH] rbd: Remove VLA usage
As part of the effort to remove VLAs from the kernel[1], this moves
the literal values into the stack array calculation instead of using a
variable for the sizing. The resulting size can be found from
sizeof(buf).
[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by: Kyle Spiers <kyle@spiers.me>
---
drivers/block/rbd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 8e40da0..0e94e1f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3100,8 +3100,8 @@ static int __rbd_notify_op_lock(struct rbd_device
*rbd_dev,
{
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
struct rbd_client_id cid = rbd_get_cid(rbd_dev);
- int buf_size = 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN;
- char buf[buf_size];
+ char buf[4 + 4 + 8 + 8 + CEPH_ENCODING_START_BLK_LEN];
+ int buf_size = sizeof(buf);
void *p = buf;
dout("%s rbd_dev %p notify_op %d\n", __func__, rbd_dev, notify_op);
@@ -3619,8 +3619,8 @@ static void __rbd_acknowledge_notify(struct
rbd_device *rbd_dev,
u64 notify_id, u64 cookie, s32 *result)
{
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
- int buf_size = 4 + CEPH_ENCODING_START_BLK_LEN;
- char buf[buf_size];
+ char buf[4 + CEPH_ENCODING_START_BLK_LEN];
+ int buf_size = sizeof(buf);
int ret;
if (result) {
-- 2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-30 20:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 19:17 [PATCH] rbd: remove VLA usage Gustavo A. R. Silva
2018-03-30 20:29 ` Ilya Dryomov
2018-03-30 20:34 ` Gustavo A. R. Silva
-- strict thread matches above, loose matches on Subject: below --
2018-03-10 5:14 [PATCH] rbd: Remove " Kyle Spiers
2018-03-10 5:14 ` Kyle Spiers
2018-03-10 15:24 ` Kees Cook
2018-03-12 9:45 ` Ilya Dryomov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.