All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 10:39 ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2017-06-14 10:39 UTC (permalink / raw)
  To: Santosh Shilimkar, Avinash Repaka
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
is expecting error pointers so it results in a NULL dereference.

Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
index 48332a6ed738..74a66cc162ed 100644
--- a/net/rds/ib_frmr.c
+++ b/net/rds/ib_frmr.c
@@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
 	struct rds_ib_mr_pool *pool;
 	struct rds_ib_mr *ibmr = NULL;
 	struct rds_ib_frmr *frmr;
-	int err = 0;
+	int err;
 
 	if (npages <= RDS_MR_8K_MSG_SIZE)
 		pool = rds_ibdev->mr_8k_pool;
@@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
 			 pool->fmr_attr.max_pages);
 	if (IS_ERR(frmr->mr)) {
 		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
+		err = -ENOMEM;
 		goto out_no_cigar;
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 10:39 ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2017-06-14 10:39 UTC (permalink / raw)
  To: Santosh Shilimkar, Avinash Repaka
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
is expecting error pointers so it results in a NULL dereference.

Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
index 48332a6ed738..74a66cc162ed 100644
--- a/net/rds/ib_frmr.c
+++ b/net/rds/ib_frmr.c
@@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
 	struct rds_ib_mr_pool *pool;
 	struct rds_ib_mr *ibmr = NULL;
 	struct rds_ib_frmr *frmr;
-	int err = 0;
+	int err;
 
 	if (npages <= RDS_MR_8K_MSG_SIZE)
 		pool = rds_ibdev->mr_8k_pool;
@@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
 			 pool->fmr_attr.max_pages);
 	if (IS_ERR(frmr->mr)) {
 		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
+		err = -ENOMEM;
 		goto out_no_cigar;
 	}
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
       [not found] ` <20170614103924.GK29394-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
@ 2017-06-14 12:54     ` Yuval Shaia
  0 siblings, 0 replies; 14+ messages in thread
From: Yuval Shaia @ 2017-06-14 12:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Santosh Shilimkar, Avinash Repaka,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> is expecting error pointers so it results in a NULL dereference.
> 
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> 
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 48332a6ed738..74a66cc162ed 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>  	struct rds_ib_mr_pool *pool;
>  	struct rds_ib_mr *ibmr = NULL;
>  	struct rds_ib_frmr *frmr;
> -	int err = 0;
> +	int err;

Can we trust it'll be zero?

>  
>  	if (npages <= RDS_MR_8K_MSG_SIZE)
>  		pool = rds_ibdev->mr_8k_pool;
> @@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>  			 pool->fmr_attr.max_pages);
>  	if (IS_ERR(frmr->mr)) {
>  		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
> +		err = -ENOMEM;
>  		goto out_no_cigar;
>  	}
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 12:54     ` Yuval Shaia
  0 siblings, 0 replies; 14+ messages in thread
From: Yuval Shaia @ 2017-06-14 12:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Santosh Shilimkar, Avinash Repaka,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> is expecting error pointers so it results in a NULL dereference.
> 
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 48332a6ed738..74a66cc162ed 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>  	struct rds_ib_mr_pool *pool;
>  	struct rds_ib_mr *ibmr = NULL;
>  	struct rds_ib_frmr *frmr;
> -	int err = 0;
> +	int err;

Can we trust it'll be zero?

>  
>  	if (npages <= RDS_MR_8K_MSG_SIZE)
>  		pool = rds_ibdev->mr_8k_pool;
> @@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>  			 pool->fmr_attr.max_pages);
>  	if (IS_ERR(frmr->mr)) {
>  		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
> +		err = -ENOMEM;
>  		goto out_no_cigar;
>  	}
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
  2017-06-14 12:54     ` Yuval Shaia
@ 2017-06-14 13:03       ` Dan Carpenter
  -1 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2017-06-14 13:03 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: Santosh Shilimkar, Avinash Repaka,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Wed, Jun 14, 2017 at 03:54:24PM +0300, Yuval Shaia wrote:
> On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> > We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> > is expecting error pointers so it results in a NULL dereference.
> > 
> > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > 
> > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > index 48332a6ed738..74a66cc162ed 100644
> > --- a/net/rds/ib_frmr.c
> > +++ b/net/rds/ib_frmr.c
> > @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> >  	struct rds_ib_mr_pool *pool;
> >  	struct rds_ib_mr *ibmr = NULL;
> >  	struct rds_ib_frmr *frmr;
> > -	int err = 0;
> > +	int err;
> 
> Can we trust it'll be zero?

We don't ever want it to be zero.  This way, hopefully, GCC will catch
it if we introduce any new bugs where we forget to set it to negative.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 13:03       ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2017-06-14 13:03 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: Santosh Shilimkar, Avinash Repaka,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Wed, Jun 14, 2017 at 03:54:24PM +0300, Yuval Shaia wrote:
> On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> > We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> > is expecting error pointers so it results in a NULL dereference.
> > 
> > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > index 48332a6ed738..74a66cc162ed 100644
> > --- a/net/rds/ib_frmr.c
> > +++ b/net/rds/ib_frmr.c
> > @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> >  	struct rds_ib_mr_pool *pool;
> >  	struct rds_ib_mr *ibmr = NULL;
> >  	struct rds_ib_frmr *frmr;
> > -	int err = 0;
> > +	int err;
> 
> Can we trust it'll be zero?

We don't ever want it to be zero.  This way, hopefully, GCC will catch
it if we introduce any new bugs where we forget to set it to negative.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
  2017-06-14 12:54     ` Yuval Shaia
@ 2017-06-14 13:05       ` Julia Lawall
  -1 siblings, 0 replies; 14+ messages in thread
From: Julia Lawall @ 2017-06-14 13:05 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: Dan Carpenter, Santosh Shilimkar, Avinash Repaka,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA



On Wed, 14 Jun 2017, Yuval Shaia wrote:

> On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> > We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> > is expecting error pointers so it results in a NULL dereference.
> >
> > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> >
> > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > index 48332a6ed738..74a66cc162ed 100644
> > --- a/net/rds/ib_frmr.c
> > +++ b/net/rds/ib_frmr.c
> > @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> >  	struct rds_ib_mr_pool *pool;
> >  	struct rds_ib_mr *ibmr = NULL;
> >  	struct rds_ib_frmr *frmr;
> > -	int err = 0;
> > +	int err;
>
> Can we trust it'll be zero?

No need.  With the change below there is always an assignment before the
only reference at the end of the function.

julia

>
> >
> >  	if (npages <= RDS_MR_8K_MSG_SIZE)
> >  		pool = rds_ibdev->mr_8k_pool;
> > @@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> >  			 pool->fmr_attr.max_pages);
> >  	if (IS_ERR(frmr->mr)) {
> >  		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
> > +		err = -ENOMEM;
> >  		goto out_no_cigar;
> >  	}
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 13:05       ` Julia Lawall
  0 siblings, 0 replies; 14+ messages in thread
From: Julia Lawall @ 2017-06-14 13:05 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: Dan Carpenter, Santosh Shilimkar, Avinash Repaka,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA



On Wed, 14 Jun 2017, Yuval Shaia wrote:

> On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> > We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> > is expecting error pointers so it results in a NULL dereference.
> >
> > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > index 48332a6ed738..74a66cc162ed 100644
> > --- a/net/rds/ib_frmr.c
> > +++ b/net/rds/ib_frmr.c
> > @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> >  	struct rds_ib_mr_pool *pool;
> >  	struct rds_ib_mr *ibmr = NULL;
> >  	struct rds_ib_frmr *frmr;
> > -	int err = 0;
> > +	int err;
>
> Can we trust it'll be zero?

No need.  With the change below there is always an assignment before the
only reference at the end of the function.

julia

>
> >
> >  	if (npages <= RDS_MR_8K_MSG_SIZE)
> >  		pool = rds_ibdev->mr_8k_pool;
> > @@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> >  			 pool->fmr_attr.max_pages);
> >  	if (IS_ERR(frmr->mr)) {
> >  		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
> > +		err = -ENOMEM;
> >  		goto out_no_cigar;
> >  	}
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
  2017-06-14 13:03       ` Dan Carpenter
@ 2017-06-14 13:05         ` Yuval Shaia
  -1 siblings, 0 replies; 14+ messages in thread
From: Yuval Shaia @ 2017-06-14 13:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Santosh Shilimkar, Avinash Repaka, linux-rdma, rds-devel,
	kernel-janitors

On Wed, Jun 14, 2017 at 04:03:40PM +0300, Dan Carpenter wrote:
> On Wed, Jun 14, 2017 at 03:54:24PM +0300, Yuval Shaia wrote:
> > On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> > > We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> > > is expecting error pointers so it results in a NULL dereference.
> > > 
> > > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > 
> > > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > > index 48332a6ed738..74a66cc162ed 100644
> > > --- a/net/rds/ib_frmr.c
> > > +++ b/net/rds/ib_frmr.c
> > > @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> > >  	struct rds_ib_mr_pool *pool;
> > >  	struct rds_ib_mr *ibmr = NULL;
> > >  	struct rds_ib_frmr *frmr;
> > > -	int err = 0;
> > > +	int err;
> > 
> > Can we trust it'll be zero?
> 
> We don't ever want it to be zero.  This way, hopefully, GCC will catch
> it if we introduce any new bugs where we forget to set it to negative.

I see your point, in a "good" case we just return the ptr.

Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>

> 
> regards,
> dan carpenter
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 13:05         ` Yuval Shaia
  0 siblings, 0 replies; 14+ messages in thread
From: Yuval Shaia @ 2017-06-14 13:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Santosh Shilimkar, Avinash Repaka, linux-rdma, rds-devel,
	kernel-janitors

On Wed, Jun 14, 2017 at 04:03:40PM +0300, Dan Carpenter wrote:
> On Wed, Jun 14, 2017 at 03:54:24PM +0300, Yuval Shaia wrote:
> > On Wed, Jun 14, 2017 at 01:39:24PM +0300, Dan Carpenter wrote:
> > > We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> > > is expecting error pointers so it results in a NULL dereference.
> > > 
> > > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > 
> > > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > > index 48332a6ed738..74a66cc162ed 100644
> > > --- a/net/rds/ib_frmr.c
> > > +++ b/net/rds/ib_frmr.c
> > > @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
> > >  	struct rds_ib_mr_pool *pool;
> > >  	struct rds_ib_mr *ibmr = NULL;
> > >  	struct rds_ib_frmr *frmr;
> > > -	int err = 0;
> > > +	int err;
> > 
> > Can we trust it'll be zero?
> 
> We don't ever want it to be zero.  This way, hopefully, GCC will catch
> it if we introduce any new bugs where we forget to set it to negative.

I see your point, in a "good" case we just return the ptr.

Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>

> 
> regards,
> dan carpenter
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
  2017-06-14 10:39 ` Dan Carpenter
@ 2017-06-14 17:18   ` santosh.shilimkar
  -1 siblings, 0 replies; 14+ messages in thread
From: santosh.shilimkar @ 2017-06-14 17:18 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Avinash Repaka, linux-rdma, rds-devel, kernel-janitors, netdev

On 6/14/17 3:39 AM, Dan Carpenter wrote:
> We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> is expecting error pointers so it results in a NULL dereference.
>
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
Thanks Dan for fix.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

You haven't copied netdev. Can you please resend the patch
with my ack on netdev so that Dave can pick it up.

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 17:18   ` santosh.shilimkar
  0 siblings, 0 replies; 14+ messages in thread
From: santosh.shilimkar @ 2017-06-14 17:18 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Avinash Repaka, linux-rdma, rds-devel, kernel-janitors, netdev

On 6/14/17 3:39 AM, Dan Carpenter wrote:
> We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> is expecting error pointers so it results in a NULL dereference.
>
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
Thanks Dan for fix.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

You haven't copied netdev. Can you please resend the patch
with my ack on netdev so that Dave can pick it up.





^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
  2017-06-14 10:39 ` Dan Carpenter
@ 2017-06-14 20:59   ` Avinash Repaka
  -1 siblings, 0 replies; 14+ messages in thread
From: Avinash Repaka @ 2017-06-14 20:59 UTC (permalink / raw)
  To: Dan Carpenter, Santosh Shilimkar; +Cc: linux-rdma, rds-devel, kernel-janitors

Reviewed-by: Avinash Repaka <avinash.repaka@oracle.com>

On 06/14/2017 03:39 AM, Dan Carpenter wrote:
> We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> is expecting error pointers so it results in a NULL dereference.
>
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 48332a6ed738..74a66cc162ed 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>   	struct rds_ib_mr_pool *pool;
>   	struct rds_ib_mr *ibmr = NULL;
>   	struct rds_ib_frmr *frmr;
> -	int err = 0;
> +	int err;
>   
>   	if (npages <= RDS_MR_8K_MSG_SIZE)
>   		pool = rds_ibdev->mr_8k_pool;
> @@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>   			 pool->fmr_attr.max_pages);
>   	if (IS_ERR(frmr->mr)) {
>   		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
> +		err = -ENOMEM;
>   		goto out_no_cigar;
>   	}
>   


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr()
@ 2017-06-14 20:59   ` Avinash Repaka
  0 siblings, 0 replies; 14+ messages in thread
From: Avinash Repaka @ 2017-06-14 20:59 UTC (permalink / raw)
  To: Dan Carpenter, Santosh Shilimkar; +Cc: linux-rdma, rds-devel, kernel-janitors

Reviewed-by: Avinash Repaka <avinash.repaka@oracle.com>

On 06/14/2017 03:39 AM, Dan Carpenter wrote:
> We accidentally return ERR_PTR(0) if ib_alloc_mr() fails.  The caller
> is expecting error pointers so it results in a NULL dereference.
>
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 48332a6ed738..74a66cc162ed 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -38,7 +38,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>   	struct rds_ib_mr_pool *pool;
>   	struct rds_ib_mr *ibmr = NULL;
>   	struct rds_ib_frmr *frmr;
> -	int err = 0;
> +	int err;
>   
>   	if (npages <= RDS_MR_8K_MSG_SIZE)
>   		pool = rds_ibdev->mr_8k_pool;
> @@ -61,6 +61,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>   			 pool->fmr_attr.max_pages);
>   	if (IS_ERR(frmr->mr)) {
>   		pr_warn("RDS/IB: %s failed to allocate MR", __func__);
> +		err = -ENOMEM;
>   		goto out_no_cigar;
>   	}
>   


^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-06-14 20:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 10:39 [PATCH] RDS: IB: NULL dereference on error in rds_ib_alloc_frmr() Dan Carpenter
2017-06-14 10:39 ` Dan Carpenter
     [not found] ` <20170614103924.GK29394-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2017-06-14 12:54   ` Yuval Shaia
2017-06-14 12:54     ` Yuval Shaia
2017-06-14 13:03     ` Dan Carpenter
2017-06-14 13:03       ` Dan Carpenter
2017-06-14 13:05       ` Yuval Shaia
2017-06-14 13:05         ` Yuval Shaia
2017-06-14 13:05     ` Julia Lawall
2017-06-14 13:05       ` Julia Lawall
2017-06-14 17:18 ` santosh.shilimkar
2017-06-14 17:18   ` santosh.shilimkar
2017-06-14 20:59 ` Avinash Repaka
2017-06-14 20:59   ` Avinash Repaka

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.