[v2] nbd_genl_status: null check for nla_nest_start
diff mbox series

Message ID 20190729164226.22632-1-navid.emamdoost@gmail.com
State New
Headers show
Series
  • [v2] nbd_genl_status: null check for nla_nest_start
Related show

Commit Message

Navid Emamdoost July 29, 2019, 4:42 p.m. UTC
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(+)

Comments

Red Hat Product Security July 30, 2019, 5:52 a.m. UTC | #1
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
Bob Liu July 30, 2019, 6:05 a.m. UTC | #2
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>
Michal Kubecek Sept. 10, 2019, 11:35 a.m. UTC | #3
(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

Patch
diff mbox series

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