* [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
@ 2018-07-04 9:32 Dan Carpenter
2018-07-04 10:41 ` Walter Harms
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Dan Carpenter @ 2018-07-04 9:32 UTC (permalink / raw)
To: kernel-janitors
"nents" is an unsigned int, so if ib_map_mr_sg() returns a negative
error code then it's type promoted to a high unsigned int which is
treated as success.
Fixes: a060b5629ab0 ("IB/core: generic RDMA READ/WRITE API")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
index c8963e91f92a..3ee0adfb45e9 100644
--- a/drivers/infiniband/core/rw.c
+++ b/drivers/infiniband/core/rw.c
@@ -87,7 +87,7 @@ static int rdma_rw_init_one_mr(struct ib_qp *qp, u8 port_num,
}
ret = ib_map_mr_sg(reg->mr, sg, nents, &offset, PAGE_SIZE);
- if (ret < nents) {
+ if (ret < 0 || ret < nents) {
ib_mr_pool_put(qp, &qp->rdma_mrs, reg->mr);
return -EINVAL;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
@ 2018-07-04 10:41 ` Walter Harms
2018-07-04 10:48 ` Dan Carpenter
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Walter Harms @ 2018-07-04 10:41 UTC (permalink / raw)
To: kernel-janitors
Am 04.07.2018 11:32, schrieb Dan Carpenter:
> "nents" is an unsigned int, so if ib_map_mr_sg() returns a negative
> error code then it's type promoted to a high unsigned int which is
> treated as success.
>
> Fixes: a060b5629ab0 ("IB/core: generic RDMA READ/WRITE API")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
> index c8963e91f92a..3ee0adfb45e9 100644
> --- a/drivers/infiniband/core/rw.c
> +++ b/drivers/infiniband/core/rw.c
> @@ -87,7 +87,7 @@ static int rdma_rw_init_one_mr(struct ib_qp *qp, u8
> port_num,
> }
>
> ret = ib_map_mr_sg(reg->mr, sg, nents, &offset, PAGE_SIZE);
> - if (ret < nents) {
> + if (ret < 0 || ret < nents) {
> ib_mr_pool_put(qp, &qp->rdma_mrs, reg->mr);
> return -EINVAL;
> }
I do not understand the error here.
ib_map_mr_sg() obviously knows about nents and returns an invalid value ?
Or is that a special case here ?
re,
wh
> --
> 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] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
2018-07-04 10:41 ` Walter Harms
@ 2018-07-04 10:48 ` Dan Carpenter
2018-07-04 14:49 ` Jason Gunthorpe
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2018-07-04 10:48 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 12:41:53PM +0200, Walter Harms wrote:
>
> Am 04.07.2018 11:32, schrieb Dan Carpenter:
> > "nents" is an unsigned int, so if ib_map_mr_sg() returns a negative
> > error code then it's type promoted to a high unsigned int which is
> > treated as success.
> >
> > Fixes: a060b5629ab0 ("IB/core: generic RDMA READ/WRITE API")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
> > index c8963e91f92a..3ee0adfb45e9 100644
> > --- a/drivers/infiniband/core/rw.c
> > +++ b/drivers/infiniband/core/rw.c
> > @@ -87,7 +87,7 @@ static int rdma_rw_init_one_mr(struct ib_qp *qp, u8
> > port_num,
> > }
> >
> > ret = ib_map_mr_sg(reg->mr, sg, nents, &offset, PAGE_SIZE);
> > - if (ret < nents) {
> > + if (ret < 0 || ret < nents) {
> > ib_mr_pool_put(qp, &qp->rdma_mrs, reg->mr);
> > return -EINVAL;
> > }
>
> I do not understand the error here.
>
> ib_map_mr_sg() obviously knows about nents and returns an invalid value ?
> Or is that a special case here ?
>
ib_map_mr_sg() can return -ENOMEM, -EINVAL, other error codes or a
positive number <= to nents. If it returns -ENOMEM then that's type
promoted to a high positive value (more than nents for sure) and treated
as success.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
2018-07-04 10:41 ` Walter Harms
2018-07-04 10:48 ` Dan Carpenter
@ 2018-07-04 14:49 ` Jason Gunthorpe
2018-07-04 16:27 ` Leon Romanovsky
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2018-07-04 14:49 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 12:55:41PM +0200, HÃ¥kon Bugge wrote:
> Is:
>
> if (ret < (int)nents) {
>
> a more intuitive fix?
That could lead to truncation/force negativeness of nents :(
I wonder how many bugs like this we have.
Jason
--
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] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
` (2 preceding siblings ...)
2018-07-04 14:49 ` Jason Gunthorpe
@ 2018-07-04 16:27 ` Leon Romanovsky
2018-07-04 17:01 ` Dan Carpenter
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2018-07-04 16:27 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 728 bytes --]
On Wed, Jul 04, 2018 at 08:49:47AM -0600, Jason Gunthorpe wrote:
> On Wed, Jul 04, 2018 at 12:55:41PM +0200, Håkon Bugge wrote:
> > Is:
> >
> > if (ret < (int)nents) {
> >
> > a more intuitive fix?
>
> That could lead to truncation/force negativeness of nents :(
IS_ERROR_VALUE() from include/linux/err.h?
>
> I wonder how many bugs like this we have.
Judging by the number of similar patches in netdev, I don't think
that we will have many "bugs", but maybe I'm wrong about it.
>
> Jason
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
` (3 preceding siblings ...)
2018-07-04 16:27 ` Leon Romanovsky
@ 2018-07-04 17:01 ` Dan Carpenter
2018-07-04 17:07 ` Dan Carpenter
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2018-07-04 17:01 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 08:49:47AM -0600, Jason Gunthorpe wrote:
> On Wed, Jul 04, 2018 at 12:55:41PM +0200, Håkon Bugge wrote:
> > Is:
> >
> > if (ret < (int)nents) {
> >
> > a more intuitive fix?
>
> That could lead to truncation/force negativeness of nents :(
>
In this case, if nents is over INT_MAX we're already toasted.
> I wonder how many bugs like this we have.
This is a static checker fix, so Julia fixed 3 and I fixed 6...
regards,
dan carpenter
--
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] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
` (4 preceding siblings ...)
2018-07-04 17:01 ` Dan Carpenter
@ 2018-07-04 17:07 ` Dan Carpenter
2018-07-04 18:05 ` Jason Gunthorpe
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2018-07-04 17:07 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 07:27:43PM +0300, Leon Romanovsky wrote:
> On Wed, Jul 04, 2018 at 08:49:47AM -0600, Jason Gunthorpe wrote:
> > On Wed, Jul 04, 2018 at 12:55:41PM +0200, Håkon Bugge wrote:
> > > Is:
> > >
> > > if (ret < (int)nents) {
> > >
> > > a more intuitive fix?
> >
> > That could lead to truncation/force negativeness of nents :(
>
> IS_ERROR_VALUE() from include/linux/err.h?
>
Heh. Please, no. First of all that doesn't work, but also that's not
how IS_ERROR_VALUE() is supposed to be used.
regards,
dan carpenter
--
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] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
` (5 preceding siblings ...)
2018-07-04 17:07 ` Dan Carpenter
@ 2018-07-04 18:05 ` Jason Gunthorpe
2018-07-04 18:05 ` Jason Gunthorpe
2018-07-08 15:07 ` Christoph Hellwig
8 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2018-07-04 18:05 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 08:01:57PM +0300, Dan Carpenter wrote:
> On Wed, Jul 04, 2018 at 08:49:47AM -0600, Jason Gunthorpe wrote:
> > On Wed, Jul 04, 2018 at 12:55:41PM +0200, HÃ¥kon Bugge wrote:
> > > Is:
> > >
> > > if (ret < (int)nents) {
> > >
> > > a more intuitive fix?
> >
> > That could lead to truncation/force negativeness of nents :(
> >
>
> In this case, if nents is over INT_MAX we're already toasted.
Ugh, yes, functions accepting int for unsigned values is any
alarmingly common mistake too.
> > I wonder how many bugs like this we have.
>
> This is a static checker fix, so Julia fixed 3 and I fixed 6...
Leon found another case of implicit casting creating a user space
triggerable bug last week..
Jason
--
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] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
` (6 preceding siblings ...)
2018-07-04 18:05 ` Jason Gunthorpe
@ 2018-07-04 18:05 ` Jason Gunthorpe
2018-07-08 15:07 ` Christoph Hellwig
8 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2018-07-04 18:05 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 12:32:12PM +0300, Dan Carpenter wrote:
> "nents" is an unsigned int, so if ib_map_mr_sg() returns a negative
> error code then it's type promoted to a high unsigned int which is
> treated as success.
>
> Fixes: a060b5629ab0 ("IB/core: generic RDMA READ/WRITE API")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to for-next
Thanks,
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr()
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
` (7 preceding siblings ...)
2018-07-04 18:05 ` Jason Gunthorpe
@ 2018-07-08 15:07 ` Christoph Hellwig
8 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2018-07-08 15:07 UTC (permalink / raw)
To: kernel-janitors
On Wed, Jul 04, 2018 at 12:32:12PM +0300, Dan Carpenter wrote:
> "nents" is an unsigned int, so if ib_map_mr_sg() returns a negative
> error code then it's type promoted to a high unsigned int which is
> treated as success.
>
> Fixes: a060b5629ab0 ("IB/core: generic RDMA READ/WRITE API")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-07-08 15:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-04 9:32 [PATCH] IB/core: type promotion bug in rdma_rw_init_one_mr() Dan Carpenter
2018-07-04 10:41 ` Walter Harms
2018-07-04 10:48 ` Dan Carpenter
2018-07-04 14:49 ` Jason Gunthorpe
2018-07-04 16:27 ` Leon Romanovsky
2018-07-04 17:01 ` Dan Carpenter
2018-07-04 17:07 ` Dan Carpenter
2018-07-04 18:05 ` Jason Gunthorpe
2018-07-04 18:05 ` Jason Gunthorpe
2018-07-08 15:07 ` Christoph Hellwig
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.