linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / 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; 11+ messages in thread
From: Navid Emamdoost @ 2019-07-23 23:01 UTC (permalink / raw)
  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 related	[flat|nested] 11+ 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; 11+ 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] 11+ 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; 11+ 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 related	[flat|nested] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
  2019-10-17  2:17       ` [PATCH v2] " Navid Emamdoost
  2 siblings, 2 replies; 11+ 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] 11+ 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
  2019-10-21  6:42         ` Michal Kubecek
  2019-10-17  2:17       ` [PATCH v2] " Navid Emamdoost
  1 sibling, 1 reply; 11+ 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 related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] nbd_genl_status: null check for nla_nest_start
  2019-09-10 11:35     ` Michal Kubecek
  2019-09-11 16:40       ` [PATCH v3] " Navid Emamdoost
@ 2019-10-17  2:17       ` Navid Emamdoost
  2019-10-17 19:37         ` [engineering.redhat.com #498403] " Red Hat Product Security
  1 sibling, 1 reply; 11+ messages in thread
From: Navid Emamdoost @ 2019-10-17  2:17 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: Josef Bacik, Kangjie Lu, Stephen McCamant, secalert,
	Navid Emamdoost, Jens Axboe, linux-block, nbd, LKML

Hi Michal, please check v3 at https://lore.kernel.org/patchwork/patch/1126650/


Thanks,
Navid.

On Tue, Sep 10, 2019 at 6:35 AM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> (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



-- 
Navid.

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

* [engineering.redhat.com #498403] Re: [PATCH v2] nbd_genl_status: null check for nla_nest_start
  2019-10-17  2:17       ` [PATCH v2] " Navid Emamdoost
@ 2019-10-17 19:37         ` Red Hat Product Security
  0 siblings, 0 replies; 11+ messages in thread
From: Red Hat Product Security @ 2019-10-17 19:37 UTC (permalink / raw)
  To: navid.emamdoost
  Cc: axboe, emamd001, josef, kjlu, linux-block, linux-kernel,
	mkubecek, nbd, smccaman

Hi Navid,

Not sure if you meant to cc secalert@redhat.com on this. If anything is needed
from our side please let us know!

On Wed Oct 16 22:17:42 2019, navid.emamdoost@gmail.com wrote:
> Hi Michal, please check v3 at
> https://lore.kernel.org/patchwork/patch/1126650/
>
>
> Thanks,
> Navid.
>
> On Tue, Sep 10, 2019 at 6:35 AM Michal Kubecek <mkubecek@suse.cz>
> wrote:
> >
> > (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
>
>
>


--
Kat Bost
Red Hat Product Security


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

* Re: [PATCH v3] nbd_genl_status: null check for nla_nest_start
  2019-09-11 16:40       ` [PATCH v3] " Navid Emamdoost
@ 2019-10-21  6:42         ` Michal Kubecek
  2021-04-14  3:05           ` Mark-PK Tsai
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Kubecek @ 2019-10-21  6:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: Navid Emamdoost, emamd001, smccaman, kjlu, Josef Bacik,
	Jens Axboe, linux-block, nbd

On Wed, Sep 11, 2019 at 11:40:12AM -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.
> 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) {

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

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

* Re: [PATCH v3] nbd_genl_status: null check for nla_nest_start
  2019-10-21  6:42         ` Michal Kubecek
@ 2021-04-14  3:05           ` Mark-PK Tsai
  0 siblings, 0 replies; 11+ messages in thread
From: Mark-PK Tsai @ 2021-04-14  3:05 UTC (permalink / raw)
  To: mkubecek, linux-kernel
  Cc: axboe, emamd001, josef, kjlu, linux-block, navid.emamdoost, nbd,
	smccaman, mark-pk.tsai, jy.ho, yj.chiang

From: Michal Kubecek <mkubecek@suse.cz>

Hi,

I found that CVE-2019-16089 still exist in upstream kernel.
Does anyone know why this patch was not merged?



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

end of thread, other threads:[~2021-04-14  3:05 UTC | newest]

Thread overview: 11+ 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
2019-10-21  6:42         ` Michal Kubecek
2021-04-14  3:05           ` Mark-PK Tsai
2019-10-17  2:17       ` [PATCH v2] " Navid Emamdoost
2019-10-17 19:37         ` [engineering.redhat.com #498403] " Red Hat Product Security

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).