* [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops
@ 2017-05-09 13:31 Colin King
2017-05-09 14:04 ` Dan Carpenter
2017-05-09 19:57 ` Christoph Hellwig
0 siblings, 2 replies; 6+ messages in thread
From: Colin King @ 2017-05-09 13:31 UTC (permalink / raw)
To: Ari Kauppi, J . Bruce Fields, Jeff Layton, linux-nfs
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Array nfsd4_layout_ops has LAYOUT_TYPE_MAX elements (which is currently
just 6), so check for this upper bound rather than the hard coded upper
bound of 32 to avoid an out of bounds read on array nfsd4_layout_ops.
Detected by CoverityScan, CID#1433518 ("Out-of-bounds read")
Fixes: e79104c9bd2d26 ("nfsd: fix undefined behavior in nfsd4_layout_verify")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
fs/nfsd/nfs4proc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 1dbf62190bee..c453a1998e00 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
return NULL;
}
- if (layout_type >= 32 || !(exp->ex_layout_types & (1 << layout_type))) {
+ if (layout_type >= LAYOUT_TYPE_MAX ||
+ !(exp->ex_layout_types & (1 << layout_type))) {
dprintk("%s: layout type %d not supported\n",
__func__, layout_type);
return NULL;
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops
2017-05-09 13:31 [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops Colin King
@ 2017-05-09 14:04 ` Dan Carpenter
2017-05-09 21:03 ` J . Bruce Fields
2017-05-09 19:57 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2017-05-09 14:04 UTC (permalink / raw)
To: Colin King
Cc: Ari Kauppi, J . Bruce Fields, Jeff Layton, linux-nfs,
kernel-janitors, linux-kernel
On Tue, May 09, 2017 at 02:31:21PM +0100, Colin King wrote:
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 1dbf62190bee..c453a1998e00 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
> return NULL;
> }
>
> - if (layout_type >= 32 || !(exp->ex_layout_types & (1 << layout_type))) {
> + if (layout_type >= LAYOUT_TYPE_MAX ||
> + !(exp->ex_layout_types & (1 << layout_type))) {
The 32 is there to prevent a shift wrapping bug. The bit test prevents
a buffer overflow so this can't actually overflow. But this change
doesn't hurt and is probably cleaner.
exp->ex_layout_types is set in nfsd4_setup_layout_type().
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops
2017-05-09 14:04 ` Dan Carpenter
@ 2017-05-09 21:03 ` J . Bruce Fields
2017-05-09 21:14 ` Colin Ian King
0 siblings, 1 reply; 6+ messages in thread
From: J . Bruce Fields @ 2017-05-09 21:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Colin King, Ari Kauppi, Jeff Layton, linux-nfs, kernel-janitors,
linux-kernel
On Tue, May 09, 2017 at 05:04:14PM +0300, Dan Carpenter wrote:
> On Tue, May 09, 2017 at 02:31:21PM +0100, Colin King wrote:
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 1dbf62190bee..c453a1998e00 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
> > return NULL;
> > }
> >
> > - if (layout_type >= 32 || !(exp->ex_layout_types & (1 << layout_type))) {
> > + if (layout_type >= LAYOUT_TYPE_MAX ||
> > + !(exp->ex_layout_types & (1 << layout_type))) {
>
> The 32 is there to prevent a shift wrapping bug. The bit test prevents
> a buffer overflow so this can't actually overflow.
Yes, looks like a false positive for coverity.
> But this change doesn't hurt and is probably cleaner.
Sure. Hope it's OK if I just merge this into the previous commit:
--b.
commit 16b6f81d8ed9
Author: Ari Kauppi <ari@synopsys.com>
Date: Fri May 5 16:07:55 2017 -0400
nfsd: fix undefined behavior in nfsd4_layout_verify
UBSAN: Undefined behaviour in fs/nfsd/nfs4proc.c:1262:34
shift exponent 128 is too large for 32-bit type 'int'
Depending on compiler+architecture, this may cause the check for
layout_type to succeed for overly large values (which seems to be the
case with amd64). The large value will be later used in de-referencing
nfsd4_layout_ops for function pointers.
Reported-by: Jani Tuovila <tuovila@synopsys.com>
Signed-off-by: Ari Kauppi <ari@synopsys.com>
[colin.king@canonical.com: use LAYOUT_TYPE_MAX instead of 32]
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index d86031b6ad79..c453a1998e00 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
return NULL;
}
- if (!(exp->ex_layout_types & (1 << layout_type))) {
+ if (layout_type >= LAYOUT_TYPE_MAX ||
+ !(exp->ex_layout_types & (1 << layout_type))) {
dprintk("%s: layout type %d not supported\n",
__func__, layout_type);
return NULL;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops
2017-05-09 21:03 ` J . Bruce Fields
@ 2017-05-09 21:14 ` Colin Ian King
2017-05-10 5:24 ` Ari Kauppi
0 siblings, 1 reply; 6+ messages in thread
From: Colin Ian King @ 2017-05-09 21:14 UTC (permalink / raw)
To: J . Bruce Fields, Dan Carpenter
Cc: Ari Kauppi, Jeff Layton, linux-nfs, kernel-janitors, linux-kernel
On 09/05/17 22:03, J . Bruce Fields wrote:
> On Tue, May 09, 2017 at 05:04:14PM +0300, Dan Carpenter wrote:
>> On Tue, May 09, 2017 at 02:31:21PM +0100, Colin King wrote:
>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>> index 1dbf62190bee..c453a1998e00 100644
>>> --- a/fs/nfsd/nfs4proc.c
>>> +++ b/fs/nfsd/nfs4proc.c
>>> @@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
>>> return NULL;
>>> }
>>>
>>> - if (layout_type >= 32 || !(exp->ex_layout_types & (1 << layout_type))) {
>>> + if (layout_type >= LAYOUT_TYPE_MAX ||
>>> + !(exp->ex_layout_types & (1 << layout_type))) {
>>
>> The 32 is there to prevent a shift wrapping bug. The bit test prevents
>> a buffer overflow so this can't actually overflow.
>
> Yes, looks like a false positive for coverity.
>
>> But this change doesn't hurt and is probably cleaner.
>
> Sure. Hope it's OK if I just merge this into the previous commit:
Fine by me. Colin
>
> --b.
>
> commit 16b6f81d8ed9
> Author: Ari Kauppi <ari@synopsys.com>
> Date: Fri May 5 16:07:55 2017 -0400
>
> nfsd: fix undefined behavior in nfsd4_layout_verify
>
> UBSAN: Undefined behaviour in fs/nfsd/nfs4proc.c:1262:34
> shift exponent 128 is too large for 32-bit type 'int'
>
> Depending on compiler+architecture, this may cause the check for
> layout_type to succeed for overly large values (which seems to be the
> case with amd64). The large value will be later used in de-referencing
> nfsd4_layout_ops for function pointers.
>
> Reported-by: Jani Tuovila <tuovila@synopsys.com>
> Signed-off-by: Ari Kauppi <ari@synopsys.com>
> [colin.king@canonical.com: use LAYOUT_TYPE_MAX instead of 32]
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index d86031b6ad79..c453a1998e00 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
> return NULL;
> }
>
> - if (!(exp->ex_layout_types & (1 << layout_type))) {
> + if (layout_type >= LAYOUT_TYPE_MAX ||
> + !(exp->ex_layout_types & (1 << layout_type))) {
> dprintk("%s: layout type %d not supported\n",
> __func__, layout_type);
> return NULL;
> --
> 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] 6+ messages in thread* Re: [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops
2017-05-09 21:14 ` Colin Ian King
@ 2017-05-10 5:24 ` Ari Kauppi
0 siblings, 0 replies; 6+ messages in thread
From: Ari Kauppi @ 2017-05-10 5:24 UTC (permalink / raw)
To: Colin Ian King, J . Bruce Fields, Dan Carpenter
Cc: Ari Kauppi, Jeff Layton, linux-nfs, kernel-janitors, linux-kernel
> On 10.5.2017, at 0.14, Colin Ian King <colin.king@canonical.com> wrote:
>
> On 09/05/17 22:03, J . Bruce Fields wrote:
>> On Tue, May 09, 2017 at 05:04:14PM +0300, Dan Carpenter wrote:
>>> On Tue, May 09, 2017 at 02:31:21PM +0100, Colin King wrote:
>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>> index 1dbf62190bee..c453a1998e00 100644
>>>> --- a/fs/nfsd/nfs4proc.c
>>>> +++ b/fs/nfsd/nfs4proc.c
>>>> @@ -1259,7 +1259,8 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
>>>> return NULL;
>>>> }
>>>>
>>>> - if (layout_type >= 32 || !(exp->ex_layout_types & (1 << layout_type))) {
>>>> + if (layout_type >= LAYOUT_TYPE_MAX ||
>>>> + !(exp->ex_layout_types & (1 << layout_type))) {
>>>
>>> The 32 is there to prevent a shift wrapping bug. The bit test prevents
>>> a buffer overflow so this can't actually overflow.
>>
>> Yes, looks like a false positive for coverity.
>>
>>> But this change doesn't hurt and is probably cleaner.
>>
>> Sure. Hope it's OK if I just merge this into the previous commit:
>
> Fine by me. Colin
Looks good to me.
Thanks,
--
Ari
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops
2017-05-09 13:31 [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops Colin King
2017-05-09 14:04 ` Dan Carpenter
@ 2017-05-09 19:57 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2017-05-09 19:57 UTC (permalink / raw)
To: Colin King
Cc: Ari Kauppi, J . Bruce Fields, Jeff Layton, linux-nfs,
kernel-janitors, linux-kernel
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-10 5:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-09 13:31 [PATCH] nfsd: avoid out of bounds read on array nfsd4_layout_ops Colin King
2017-05-09 14:04 ` Dan Carpenter
2017-05-09 21:03 ` J . Bruce Fields
2017-05-09 21:14 ` Colin Ian King
2017-05-10 5:24 ` Ari Kauppi
2017-05-09 19:57 ` Christoph Hellwig
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).