* [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config()
@ 2020-12-02 15:26 Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-02 15:26 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Stefan Hajnoczi, Marc-André Lureau, Max Reitz,
Stefano Garzarella
v2:
* Print errors [Marc-André]
Markus Armbruster pointed out that g_return_val_if() is meant for programming
errors. It must not be used for input validation since it can be compiled out.
Use explicit if statements instead.
This patch series converts vhost-user device backends that use
g_return_val_if() in get/set_config().
Stefan Hajnoczi (4):
contrib/vhost-user-blk: avoid g_return_val_if() input validation
contrib/vhost-user-gpu: avoid g_return_val_if() input validation
contrib/vhost-user-input: avoid g_return_val_if() input validation
block/export: avoid g_return_val_if() input validation
block/export/vhost-user-blk-server.c | 6 +++++-
contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
contrib/vhost-user-input/main.c | 6 +++++-
4 files changed, 20 insertions(+), 4 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation
2020-12-02 15:26 [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefan Hajnoczi
@ 2020-12-02 15:26 ` Stefan Hajnoczi
2020-12-02 15:49 ` Marc-André Lureau
` (2 more replies)
2020-12-02 15:26 ` [PATCH v2 2/4] contrib/vhost-user-gpu: " Stefan Hajnoczi
` (3 subsequent siblings)
4 siblings, 3 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-02 15:26 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Stefan Hajnoczi, Marc-André Lureau, Max Reitz,
Stefano Garzarella
Do not validate input with g_return_val_if(). This API is intended for
checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
Use an explicit if statement for input validation so it cannot
accidentally be compiled out.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index dc981bf945..60e3c9ed37 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -404,7 +404,11 @@ vub_get_config(VuDev *vu_dev, uint8_t *config, uint32_t len)
VugDev *gdev;
VubDev *vdev_blk;
- g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1);
+ if (len > sizeof(struct virtio_blk_config)) {
+ fprintf(stderr, "Invalid get_config len %u, expected <= %zu\n",
+ len, sizeof(struct virtio_blk_config));
+ return -1;
+ }
gdev = container_of(vu_dev, VugDev, parent);
vdev_blk = container_of(gdev, VubDev, parent);
--
2.28.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation
2020-12-02 15:26 [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
@ 2020-12-02 15:26 ` Stefan Hajnoczi
2020-12-02 15:50 ` Marc-André Lureau
2020-12-02 15:26 ` [PATCH v2 3/4] contrib/vhost-user-input: " Stefan Hajnoczi
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-02 15:26 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Stefan Hajnoczi, Marc-André Lureau, Max Reitz,
Stefano Garzarella
Do not validate input with g_return_val_if(). This API is intended for
checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
Use an explicit if statement for input validation so it cannot
accidentally be compiled out.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c
index a019d0a9ac..534bad24d1 100644
--- a/contrib/vhost-user-gpu/vhost-user-gpu.c
+++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
@@ -1044,7 +1044,11 @@ vg_get_config(VuDev *dev, uint8_t *config, uint32_t len)
{
VuGpu *g = container_of(dev, VuGpu, dev.parent);
- g_return_val_if_fail(len <= sizeof(struct virtio_gpu_config), -1);
+ if (len > sizeof(struct virtio_gpu_config)) {
+ g_critical("%s: len %u is larger than %zu",
+ __func__, len, sizeof(struct virtio_gpu_config));
+ return -1;
+ }
if (opt_virgl) {
g->virtio_config.num_capsets = vg_virgl_get_num_capsets();
--
2.28.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/4] contrib/vhost-user-input: avoid g_return_val_if() input validation
2020-12-02 15:26 [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 2/4] contrib/vhost-user-gpu: " Stefan Hajnoczi
@ 2020-12-02 15:26 ` Stefan Hajnoczi
2020-12-02 15:51 ` Marc-André Lureau
2020-12-02 15:26 ` [PATCH v2 4/4] block/export: " Stefan Hajnoczi
2020-12-03 13:41 ` [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefano Garzarella
4 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-02 15:26 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Stefan Hajnoczi, Marc-André Lureau, Max Reitz,
Stefano Garzarella
Do not validate input with g_return_val_if(). This API is intended for
checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
Use an explicit if statement for input validation so it cannot
accidentally be compiled out.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
contrib/vhost-user-input/main.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
index 6020c6f33a..1d79c61200 100644
--- a/contrib/vhost-user-input/main.c
+++ b/contrib/vhost-user-input/main.c
@@ -212,7 +212,11 @@ static int vi_get_config(VuDev *dev, uint8_t *config, uint32_t len)
{
VuInput *vi = container_of(dev, VuInput, dev.parent);
- g_return_val_if_fail(len <= sizeof(*vi->sel_config), -1);
+ if (len > sizeof(*vi->sel_config)) {
+ g_critical("%s: len %u is larger than %zu",
+ __func__, len, sizeof(*vi->sel_config));
+ return -1;
+ }
if (vi->sel_config) {
memcpy(config, vi->sel_config, len);
--
2.28.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/4] block/export: avoid g_return_val_if() input validation
2020-12-02 15:26 [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefan Hajnoczi
` (2 preceding siblings ...)
2020-12-02 15:26 ` [PATCH v2 3/4] contrib/vhost-user-input: " Stefan Hajnoczi
@ 2020-12-02 15:26 ` Stefan Hajnoczi
2020-12-02 15:52 ` Marc-André Lureau
2020-12-02 15:58 ` Philippe Mathieu-Daudé
2020-12-03 13:41 ` [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefano Garzarella
4 siblings, 2 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-02 15:26 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Stefan Hajnoczi, Marc-André Lureau, Max Reitz,
Stefano Garzarella
Do not validate input with g_return_val_if(). This API is intended for
checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
Use an explicit if statement for input validation so it cannot
accidentally be compiled out.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/export/vhost-user-blk-server.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 62672d1cb9..bccbc98d57 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -267,7 +267,11 @@ vu_blk_get_config(VuDev *vu_dev, uint8_t *config, uint32_t len)
VuServer *server = container_of(vu_dev, VuServer, vu_dev);
VuBlkExport *vexp = container_of(server, VuBlkExport, vu_server);
- g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1);
+ if (len > sizeof(struct virtio_blk_config)) {
+ error_report("Invalid get_config len %u, expected <= %zu",
+ len, sizeof(struct virtio_blk_config));
+ return -1;
+ }
memcpy(config, &vexp->blkcfg, len);
return 0;
--
2.28.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
@ 2020-12-02 15:49 ` Marc-André Lureau
2020-12-02 15:54 ` Raphael Norwitz
2020-12-02 15:55 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2020-12-02 15:49 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]
On Wed, Dec 2, 2020 at 7:26 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
> contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
> b/contrib/vhost-user-blk/vhost-user-blk.c
> index dc981bf945..60e3c9ed37 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -404,7 +404,11 @@ vub_get_config(VuDev *vu_dev, uint8_t *config,
> uint32_t len)
> VugDev *gdev;
> VubDev *vdev_blk;
>
> - g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1);
> + if (len > sizeof(struct virtio_blk_config)) {
> + fprintf(stderr, "Invalid get_config len %u, expected <= %zu\n",
> + len, sizeof(struct virtio_blk_config));
> + return -1;
> + }
>
> gdev = container_of(vu_dev, VugDev, parent);
> vdev_blk = container_of(gdev, VubDev, parent);
> --
> 2.28.0
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2306 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 2/4] contrib/vhost-user-gpu: " Stefan Hajnoczi
@ 2020-12-02 15:50 ` Marc-André Lureau
2020-12-03 9:51 ` Stefan Hajnoczi
0 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2020-12-02 15:50 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]
On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c
> b/contrib/vhost-user-gpu/vhost-user-gpu.c
> index a019d0a9ac..534bad24d1 100644
> --- a/contrib/vhost-user-gpu/vhost-user-gpu.c
> +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
> @@ -1044,7 +1044,11 @@ vg_get_config(VuDev *dev, uint8_t *config, uint32_t
> len)
> {
> VuGpu *g = container_of(dev, VuGpu, dev.parent);
>
> - g_return_val_if_fail(len <= sizeof(struct virtio_gpu_config), -1);
> + if (len > sizeof(struct virtio_gpu_config)) {
> + g_critical("%s: len %u is larger than %zu",
> + __func__, len, sizeof(struct virtio_gpu_config));
>
g_critical() already has __FILE__ __LINE__ and G_STRFUNC.
otherwise looks good:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
+ return -1;
> + }
>
> if (opt_virgl) {
> g->virtio_config.num_capsets = vg_virgl_get_num_capsets();
> --
> 2.28.0
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2415 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] contrib/vhost-user-input: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 3/4] contrib/vhost-user-input: " Stefan Hajnoczi
@ 2020-12-02 15:51 ` Marc-André Lureau
2020-12-03 9:52 ` Stefan Hajnoczi
0 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2020-12-02 15:51 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> contrib/vhost-user-input/main.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-input/main.c
> b/contrib/vhost-user-input/main.c
> index 6020c6f33a..1d79c61200 100644
> --- a/contrib/vhost-user-input/main.c
> +++ b/contrib/vhost-user-input/main.c
> @@ -212,7 +212,11 @@ static int vi_get_config(VuDev *dev, uint8_t *config,
> uint32_t len)
> {
> VuInput *vi = container_of(dev, VuInput, dev.parent);
>
> - g_return_val_if_fail(len <= sizeof(*vi->sel_config), -1);
> + if (len > sizeof(*vi->sel_config)) {
> + g_critical("%s: len %u is larger than %zu",
> + __func__, len, sizeof(*vi->sel_config));
> + return -1;
>
g_critical() already has __FILE__ __LINE__ and G_STRFUNC.
otherwise looks good:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> + }
>
> if (vi->sel_config) {
> memcpy(config, vi->sel_config, len);
> --
> 2.28.0
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2370 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/4] block/export: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 4/4] block/export: " Stefan Hajnoczi
@ 2020-12-02 15:52 ` Marc-André Lureau
2020-12-02 15:58 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2020-12-02 15:52 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]
On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
> block/export/vhost-user-blk-server.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/block/export/vhost-user-blk-server.c
> b/block/export/vhost-user-blk-server.c
> index 62672d1cb9..bccbc98d57 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
> @@ -267,7 +267,11 @@ vu_blk_get_config(VuDev *vu_dev, uint8_t *config,
> uint32_t len)
> VuServer *server = container_of(vu_dev, VuServer, vu_dev);
> VuBlkExport *vexp = container_of(server, VuBlkExport, vu_server);
>
> - g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1);
> + if (len > sizeof(struct virtio_blk_config)) {
> + error_report("Invalid get_config len %u, expected <= %zu",
> + len, sizeof(struct virtio_blk_config));
> + return -1;
> + }
>
> memcpy(config, &vexp->blkcfg, len);
> return 0;
> --
> 2.28.0
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2351 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
2020-12-02 15:49 ` Marc-André Lureau
@ 2020-12-02 15:54 ` Raphael Norwitz
2020-12-02 15:55 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Raphael Norwitz @ 2020-12-02 15:54 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, QEMU, Max Reitz, Marc-André Lureau, Gerd Hoffmann,
Marc-André Lureau, Raphael Norwitz, Stefano Garzarella
On Wed, Dec 2, 2020 at 10:27 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> ---
> contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index dc981bf945..60e3c9ed37 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -404,7 +404,11 @@ vub_get_config(VuDev *vu_dev, uint8_t *config, uint32_t len)
> VugDev *gdev;
> VubDev *vdev_blk;
>
> - g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1);
> + if (len > sizeof(struct virtio_blk_config)) {
> + fprintf(stderr, "Invalid get_config len %u, expected <= %zu\n",
> + len, sizeof(struct virtio_blk_config));
> + return -1;
> + }
>
> gdev = container_of(vu_dev, VugDev, parent);
> vdev_blk = container_of(gdev, VubDev, parent);
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
2020-12-02 15:49 ` Marc-André Lureau
2020-12-02 15:54 ` Raphael Norwitz
@ 2020-12-02 15:55 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-02 15:55 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Marc-André Lureau, Max Reitz, Stefano Garzarella
On 12/2/20 4:26 PM, Stefan Hajnoczi wrote:
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/4] block/export: avoid g_return_val_if() input validation
2020-12-02 15:26 ` [PATCH v2 4/4] block/export: " Stefan Hajnoczi
2020-12-02 15:52 ` Marc-André Lureau
@ 2020-12-02 15:58 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-02 15:58 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
Coiby Xu, Raphael Norwitz, Marc-André Lureau, Gerd Hoffmann,
Marc-André Lureau, Max Reitz, Stefano Garzarella
On 12/2/20 4:26 PM, Stefan Hajnoczi wrote:
> Do not validate input with g_return_val_if(). This API is intended for
> checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
>
> Use an explicit if statement for input validation so it cannot
> accidentally be compiled out.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> block/export/vhost-user-blk-server.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation
2020-12-02 15:50 ` Marc-André Lureau
@ 2020-12-03 9:51 ` Stefan Hajnoczi
2020-12-03 10:26 ` Marc-André Lureau
0 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-03 9:51 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]
On Wed, Dec 02, 2020 at 07:50:51PM +0400, Marc-André Lureau wrote:
> On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> > Do not validate input with g_return_val_if(). This API is intended for
> > checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
> >
> > Use an explicit if statement for input validation so it cannot
> > accidentally be compiled out.
> >
> > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c
> > b/contrib/vhost-user-gpu/vhost-user-gpu.c
> > index a019d0a9ac..534bad24d1 100644
> > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c
> > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
> > @@ -1044,7 +1044,11 @@ vg_get_config(VuDev *dev, uint8_t *config, uint32_t
> > len)
> > {
> > VuGpu *g = container_of(dev, VuGpu, dev.parent);
> >
> > - g_return_val_if_fail(len <= sizeof(struct virtio_gpu_config), -1);
> > + if (len > sizeof(struct virtio_gpu_config)) {
> > + g_critical("%s: len %u is larger than %zu",
> > + __func__, len, sizeof(struct virtio_gpu_config));
> >
>
> g_critical() already has __FILE__ __LINE__ and G_STRFUNC.
I did this for consistency with the logging in this source file. The
other g_critical() calls in the file also print __func__.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] contrib/vhost-user-input: avoid g_return_val_if() input validation
2020-12-02 15:51 ` Marc-André Lureau
@ 2020-12-03 9:52 ` Stefan Hajnoczi
0 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-03 9:52 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]
On Wed, Dec 02, 2020 at 07:51:49PM +0400, Marc-André Lureau wrote:
> On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> > Do not validate input with g_return_val_if(). This API is intended for
> > checking programming errors and is compiled out with -DG_DISABLE_CHECKS.
> >
> > Use an explicit if statement for input validation so it cannot
> > accidentally be compiled out.
> >
> > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > contrib/vhost-user-input/main.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/vhost-user-input/main.c
> > b/contrib/vhost-user-input/main.c
> > index 6020c6f33a..1d79c61200 100644
> > --- a/contrib/vhost-user-input/main.c
> > +++ b/contrib/vhost-user-input/main.c
> > @@ -212,7 +212,11 @@ static int vi_get_config(VuDev *dev, uint8_t *config,
> > uint32_t len)
> > {
> > VuInput *vi = container_of(dev, VuInput, dev.parent);
> >
> > - g_return_val_if_fail(len <= sizeof(*vi->sel_config), -1);
> > + if (len > sizeof(*vi->sel_config)) {
> > + g_critical("%s: len %u is larger than %zu",
> > + __func__, len, sizeof(*vi->sel_config));
> > + return -1;
> >
>
> g_critical() already has __FILE__ __LINE__ and G_STRFUNC.
Will fix.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation
2020-12-03 9:51 ` Stefan Hajnoczi
@ 2020-12-03 10:26 ` Marc-André Lureau
2020-12-03 11:37 ` Stefan Hajnoczi
0 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2020-12-03 10:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
On Thu, Dec 3, 2020 at 1:52 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Wed, Dec 02, 2020 at 07:50:51PM +0400, Marc-André Lureau wrote:
> > On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> >
> > > Do not validate input with g_return_val_if(). This API is intended for
> > > checking programming errors and is compiled out with
> -DG_DISABLE_CHECKS.
> > >
> > > Use an explicit if statement for input validation so it cannot
> > > accidentally be compiled out.
> > >
> > > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > b/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > index a019d0a9ac..534bad24d1 100644
> > > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > @@ -1044,7 +1044,11 @@ vg_get_config(VuDev *dev, uint8_t *config,
> uint32_t
> > > len)
> > > {
> > > VuGpu *g = container_of(dev, VuGpu, dev.parent);
> > >
> > > - g_return_val_if_fail(len <= sizeof(struct virtio_gpu_config), -1);
> > > + if (len > sizeof(struct virtio_gpu_config)) {
> > > + g_critical("%s: len %u is larger than %zu",
> > > + __func__, len, sizeof(struct virtio_gpu_config));
> > >
> >
> > g_critical() already has __FILE__ __LINE__ and G_STRFUNC.
>
> I did this for consistency with the logging in this source file. The
> other g_critical() calls in the file also print __func__.
>
>
>
I see, nevermind then. I gave rb anyway
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2629 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation
2020-12-03 10:26 ` Marc-André Lureau
@ 2020-12-03 11:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2020-12-03 11:37 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Kevin Wolf, open list:Block layer core, Michael S. Tsirkin,
Markus Armbruster, QEMU, Coiby Xu, Raphael Norwitz,
Gerd Hoffmann, Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 2224 bytes --]
On Thu, Dec 03, 2020 at 02:26:08PM +0400, Marc-André Lureau wrote:
> On Thu, Dec 3, 2020 at 1:52 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> > On Wed, Dec 02, 2020 at 07:50:51PM +0400, Marc-André Lureau wrote:
> > > On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi <stefanha@redhat.com>
> > wrote:
> > >
> > > > Do not validate input with g_return_val_if(). This API is intended for
> > > > checking programming errors and is compiled out with
> > -DG_DISABLE_CHECKS.
> > > >
> > > > Use an explicit if statement for input validation so it cannot
> > > > accidentally be compiled out.
> > > >
> > > > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > > ---
> > > > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > > b/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > > index a019d0a9ac..534bad24d1 100644
> > > > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
> > > > @@ -1044,7 +1044,11 @@ vg_get_config(VuDev *dev, uint8_t *config,
> > uint32_t
> > > > len)
> > > > {
> > > > VuGpu *g = container_of(dev, VuGpu, dev.parent);
> > > >
> > > > - g_return_val_if_fail(len <= sizeof(struct virtio_gpu_config), -1);
> > > > + if (len > sizeof(struct virtio_gpu_config)) {
> > > > + g_critical("%s: len %u is larger than %zu",
> > > > + __func__, len, sizeof(struct virtio_gpu_config));
> > > >
> > >
> > > g_critical() already has __FILE__ __LINE__ and G_STRFUNC.
> >
> > I did this for consistency with the logging in this source file. The
> > other g_critical() calls in the file also print __func__.
> >
> >
> >
> I see, nevermind then. I gave rb anyway
Thanks! I checked now and don't see the function name printed by
g_critical() even though G_STRFUNC is captured in the header file:
** (process:693258): CRITICAL **: 11:30:18.210: test
Maybe only custom log handlers can display the function name?
So it seems the explicit __func__ approach is okay-ish :).
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config()
2020-12-02 15:26 [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefan Hajnoczi
` (3 preceding siblings ...)
2020-12-02 15:26 ` [PATCH v2 4/4] block/export: " Stefan Hajnoczi
@ 2020-12-03 13:41 ` Stefano Garzarella
2020-12-10 8:29 ` Marc-André Lureau
4 siblings, 1 reply; 18+ messages in thread
From: Stefano Garzarella @ 2020-12-03 13:41 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Markus Armbruster,
qemu-devel, Coiby Xu, Raphael Norwitz, Marc-André Lureau,
Gerd Hoffmann, Marc-André Lureau, Max Reitz
On Wed, Dec 02, 2020 at 03:26:07PM +0000, Stefan Hajnoczi wrote:
>v2:
> * Print errors [Marc-André]
>
>Markus Armbruster pointed out that g_return_val_if() is meant for programming
>errors. It must not be used for input validation since it can be compiled out.
>Use explicit if statements instead.
>
>This patch series converts vhost-user device backends that use
>g_return_val_if() in get/set_config().
>
>Stefan Hajnoczi (4):
> contrib/vhost-user-blk: avoid g_return_val_if() input validation
> contrib/vhost-user-gpu: avoid g_return_val_if() input validation
> contrib/vhost-user-input: avoid g_return_val_if() input validation
> block/export: avoid g_return_val_if() input validation
>
> block/export/vhost-user-blk-server.c | 6 +++++-
> contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
> contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
> contrib/vhost-user-input/main.c | 6 +++++-
> 4 files changed, 20 insertions(+), 4 deletions(-)
>
>--
>2.28.0
>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config()
2020-12-03 13:41 ` [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefano Garzarella
@ 2020-12-10 8:29 ` Marc-André Lureau
0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2020-12-10 8:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, open list:Block layer core, Markus Armbruster, QEMU,
Coiby Xu, Raphael Norwitz, Gerd Hoffmann, Stefan Hajnoczi,
Max Reitz, Stefano Garzarella
[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]
Hi Michael,
On Thu, Dec 3, 2020 at 5:41 PM Stefano Garzarella <sgarzare@redhat.com>
wrote:
> On Wed, Dec 02, 2020 at 03:26:07PM +0000, Stefan Hajnoczi wrote:
> >v2:
> > * Print errors [Marc-André]
> >
> >Markus Armbruster pointed out that g_return_val_if() is meant for
> programming
> >errors. It must not be used for input validation since it can be compiled
> out.
> >Use explicit if statements instead.
> >
> >This patch series converts vhost-user device backends that use
> >g_return_val_if() in get/set_config().
> >
> >Stefan Hajnoczi (4):
> > contrib/vhost-user-blk: avoid g_return_val_if() input validation
> > contrib/vhost-user-gpu: avoid g_return_val_if() input validation
> > contrib/vhost-user-input: avoid g_return_val_if() input validation
> > block/export: avoid g_return_val_if() input validation
> >
> > block/export/vhost-user-blk-server.c | 6 +++++-
> > contrib/vhost-user-blk/vhost-user-blk.c | 6 +++++-
> > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++++-
> > contrib/vhost-user-input/main.c | 6 +++++-
> > 4 files changed, 20 insertions(+), 4 deletions(-)
> >
> >--
> >2.28.0
> >
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>
You didn't collect the v2 patch series, with the received reviewed-by. Not
a big deal here, but please be more careful.
thanks
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 1997 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-12-10 8:31 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 15:26 [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 1/4] contrib/vhost-user-blk: avoid g_return_val_if() input validation Stefan Hajnoczi
2020-12-02 15:49 ` Marc-André Lureau
2020-12-02 15:54 ` Raphael Norwitz
2020-12-02 15:55 ` Philippe Mathieu-Daudé
2020-12-02 15:26 ` [PATCH v2 2/4] contrib/vhost-user-gpu: " Stefan Hajnoczi
2020-12-02 15:50 ` Marc-André Lureau
2020-12-03 9:51 ` Stefan Hajnoczi
2020-12-03 10:26 ` Marc-André Lureau
2020-12-03 11:37 ` Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 3/4] contrib/vhost-user-input: " Stefan Hajnoczi
2020-12-02 15:51 ` Marc-André Lureau
2020-12-03 9:52 ` Stefan Hajnoczi
2020-12-02 15:26 ` [PATCH v2 4/4] block/export: " Stefan Hajnoczi
2020-12-02 15:52 ` Marc-André Lureau
2020-12-02 15:58 ` Philippe Mathieu-Daudé
2020-12-03 13:41 ` [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config() Stefano Garzarella
2020-12-10 8:29 ` Marc-André Lureau
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).