Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] nbd_genl_status: null check for nla_nest_start
@ 2019-07-23 23:01 Navid Emamdoost
  2019-07-29 13:09 ` Josef Bacik
  0 siblings, 1 reply; 7+ messages in thread
From: Navid Emamdoost @ 2019-07-23 23:01 UTC (permalink / raw)
  To: unlisted-recipients:; (no To-header on input)
  Cc: emamd001, kjlu, smccaman, secalert, Navid Emamdoost, Josef Bacik,
	Jens Axboe, linux-block, nbd, linux-kernel

nla_nest_start may fail and return NULL. The check is inserted, and
errno is selected based on other call sites within the same source code.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
 drivers/block/nbd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9bcde2325893..dba362de4d91 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2149,6 +2149,12 @@ static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
+
+	if (!dev_list) {
+		ret = -EMSGSIZE;
+		goto out;
+	}
+
 	if (index == -1) {
 		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
 		if (ret) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] nbd_genl_status: null check for nla_nest_start
  2019-07-23 23:01 [PATCH] nbd_genl_status: null check for nla_nest_start Navid Emamdoost
@ 2019-07-29 13:09 ` Josef Bacik
  2019-07-29 16:42   ` [PATCH v2] " Navid Emamdoost
  0 siblings, 1 reply; 7+ messages in thread
From: Josef Bacik @ 2019-07-29 13:09 UTC (permalink / raw)
  To: Navid Emamdoost
  Cc: emamd001, kjlu, smccaman, secalert, Josef Bacik, Jens Axboe,
	linux-block, nbd, linux-kernel

On Tue, Jul 23, 2019 at 06:01:57PM -0500, Navid Emamdoost wrote:
> nla_nest_start may fail and return NULL. The check is inserted, and
> errno is selected based on other call sites within the same source code.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
>  drivers/block/nbd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 9bcde2325893..dba362de4d91 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -2149,6 +2149,12 @@ static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
>  	}
>  
>  	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
> +

No newline here, once you fix that nit you can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] nbd_genl_status: null check for nla_nest_start
  2019-07-29 13:09 ` Josef Bacik
@ 2019-07-29 16:42   ` " Navid Emamdoost
  2019-07-30  5:52     ` [engineering.redhat.com #494735] Re: [PATCH] " Red Hat Product Security
                       ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Navid Emamdoost @ 2019-07-29 16:42 UTC (permalink / raw)
  To: josef
  Cc: kjlu, smccaman, secalert, emamd001, Navid Emamdoost, Jens Axboe,
	linux-block, nbd, linux-kernel

nla_nest_start may fail and return NULL. The check is inserted, and
errno is selected based on other call sites within the same source code.
Update: removed extra new line.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
 drivers/block/nbd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9bcde2325893..2410812d1e82 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2149,6 +2149,11 @@ static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
+	if (!dev_list) {
+		ret = -EMSGSIZE;
+		goto out;
+	}
+
 	if (index == -1) {
 		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
 		if (ret) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [engineering.redhat.com #494735] Re: [PATCH] nbd_genl_status: null check for nla_nest_start
  2019-07-29 16:42   ` [PATCH v2] " Navid Emamdoost
@ 2019-07-30  5:52     ` " Red Hat Product Security
  2019-07-30  6:05     ` [PATCH v2] " Bob Liu
  2019-09-10 11:35     ` Michal Kubecek
  2 siblings, 0 replies; 7+ messages in thread
From: Red Hat Product Security @ 2019-07-30  5:52 UTC (permalink / raw)
  To: navid.emamdoost
  Cc: axboe, emamd001, josef, kjlu, linux-block, linux-kernel, nbd, smccaman

Hi Navid,

Thank you for you report. I have forwarded this to our analysis team. Once I'll
get an update on your reported vulnerability and it's patched I'll let you
know.
Please let me know if you have any questions or concerns.

On Mon Jul 29 12:42:56 2019, navid.emamdoost@gmail.com wrote:
> nla_nest_start may fail and return NULL. The check is inserted, and
> errno is selected based on other call sites within the same source
> code.
> Update: removed extra new line.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> drivers/block/nbd.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 9bcde2325893..2410812d1e82 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -2149,6 +2149,11 @@ static int nbd_genl_status(struct sk_buff *skb,
> struct genl_info *info)
> }
>
> dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
> + if (!dev_list) {
> + ret = -EMSGSIZE;
> + goto out;
> + }
> +
> if (index == -1) {
> ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
> if (ret) {


--
Best Regards,
Dhananjay Arunesh, Red Hat Product Security
7F45 FDD1 BB92 2DA8 CD05 F034 9B3D 8FE3 50EC 5D74


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] nbd_genl_status: null check for nla_nest_start
  2019-07-29 16:42   ` [PATCH v2] " Navid Emamdoost
  2019-07-30  5:52     ` [engineering.redhat.com #494735] Re: [PATCH] " Red Hat Product Security
@ 2019-07-30  6:05     ` " Bob Liu
  2019-09-10 11:35     ` Michal Kubecek
  2 siblings, 0 replies; 7+ messages in thread
From: Bob Liu @ 2019-07-30  6:05 UTC (permalink / raw)
  To: Navid Emamdoost, josef
  Cc: kjlu, smccaman, secalert, emamd001, Jens Axboe, linux-block, nbd,
	linux-kernel

On 7/30/19 12:42 AM, Navid Emamdoost wrote:
> nla_nest_start may fail and return NULL. The check is inserted, and
> errno is selected based on other call sites within the same source code.
> Update: removed extra new line.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
>  drivers/block/nbd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 9bcde2325893..2410812d1e82 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -2149,6 +2149,11 @@ static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
>  	}
>  
>  	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
> +	if (!dev_list) {
> +		ret = -EMSGSIZE;
> +		goto out;
> +	}
> +
>  	if (index == -1) {
>  		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
>  		if (ret) {
> 

Looks good to me.
Reviewed-by: Bob Liu <bob.liu@oracle.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] nbd_genl_status: null check for nla_nest_start
  2019-07-29 16:42   ` [PATCH v2] " Navid Emamdoost
  2019-07-30  5:52     ` [engineering.redhat.com #494735] Re: [PATCH] " Red Hat Product Security
  2019-07-30  6:05     ` [PATCH v2] " Bob Liu
@ 2019-09-10 11:35     ` Michal Kubecek
  2019-09-11 16:40       ` [PATCH v3] " Navid Emamdoost
  2 siblings, 1 reply; 7+ messages in thread
From: Michal Kubecek @ 2019-09-10 11:35 UTC (permalink / raw)
  To: Navid Emamdoost
  Cc: josef, kjlu, smccaman, secalert, emamd001, Jens Axboe,
	linux-block, nbd, linux-kernel

(Just stumbled upon this patch when link to it came with a CVE bug report.)

On Mon, Jul 29, 2019 at 11:42:26AM -0500, Navid Emamdoost wrote:
> nla_nest_start may fail and return NULL. The check is inserted, and
> errno is selected based on other call sites within the same source code.
> Update: removed extra new line.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> Reviewed-by: Bob Liu <bob.liu@oracle.com>
> ---
>  drivers/block/nbd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 9bcde2325893..2410812d1e82 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -2149,6 +2149,11 @@ static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
>  	}
>  
>  	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
> +	if (!dev_list) {
> +		ret = -EMSGSIZE;
> +		goto out;
> +	}
> +
>  	if (index == -1) {
>  		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
>  		if (ret) {

You should also call nlmsg_free(reply) when you bail out so that you
don't introduce a memory leak.

Michal Kubecek

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3] nbd_genl_status: null check for nla_nest_start
  2019-09-10 11:35     ` Michal Kubecek
@ 2019-09-11 16:40       ` " Navid Emamdoost
  0 siblings, 0 replies; 7+ messages in thread
From: Navid Emamdoost @ 2019-09-11 16:40 UTC (permalink / raw)
  To: mkubecek
  Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Josef Bacik,
	Jens Axboe, linux-block, nbd, linux-kernel

nla_nest_start may fail and return NULL. The check is inserted, and
errno is selected based on other call sites within the same source code.
Update: removed extra new line.
v3 Update: added release reply, thanks to Michal Kubecek for pointing
out.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
 drivers/block/nbd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index e21d2ded732b..8a9712181c2a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2149,6 +2149,12 @@ static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
+	if (!dev_list) {
+		nlmsg_free(reply);
+		ret = -EMSGSIZE;
+		goto out;
+	}
+
 	if (index == -1) {
 		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
 		if (ret) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23 23:01 [PATCH] nbd_genl_status: null check for nla_nest_start Navid Emamdoost
2019-07-29 13:09 ` Josef Bacik
2019-07-29 16:42   ` [PATCH v2] " Navid Emamdoost
2019-07-30  5:52     ` [engineering.redhat.com #494735] Re: [PATCH] " Red Hat Product Security
2019-07-30  6:05     ` [PATCH v2] " Bob Liu
2019-09-10 11:35     ` Michal Kubecek
2019-09-11 16:40       ` [PATCH v3] " Navid Emamdoost

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org linux-block@archiver.kernel.org
	public-inbox-index linux-block


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/ public-inbox