diff mbox series

keys, dns: Fix size check of V1 server-list header

Message ID 1850031.1704921100@warthog.procyon.org.uk (mailing list archive)
State New
Headers show
Series keys, dns: Fix size check of V1 server-list header | expand

Commit Message

David Howells Jan. 10, 2024, 9:11 p.m. UTC
Fix the size check added to dns_resolver_preparse() for the V1 server-list
header so that it doesn't give EINVAL if the size supplied is the same as
the size of the header struct (which should be valid).

This can be tested with:

        echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p

which will give "add_key: Invalid argument" without this fix.

Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")
Reported-by: Pengfei Xu <pengfei.xu@intel.com>
Link: https://lore.kernel.org/r/ZZ4fyY4r3rqgZL+4@xpf.sh.intel.com/
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Edward Adam Davis <eadavis@qq.com>
cc: Linus Torvalds <torvalds@linux-foundation.org>
cc: Simon Horman <horms@kernel.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jeffrey E Altman <jaltman@auristor.com>
Cc: Wang Lei <wang840925@gmail.com>
Cc: Jeff Layton <jlayton@redhat.com>
Cc: Steve French <sfrench@us.ibm.com>
Cc: Marc Dionne <marc.dionne@auristor.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
---
 net/dns_resolver/dns_key.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sedat Dilek Jan. 11, 2024, 5:58 a.m. UTC | #1
On Wed, Jan 10, 2024 at 10:12 PM David Howells <dhowells@redhat.com> wrote:
>
>
> Fix the size check added to dns_resolver_preparse() for the V1 server-list
> header so that it doesn't give EINVAL if the size supplied is the same as
> the size of the header struct (which should be valid).
>
> This can be tested with:
>
>         echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p
>
> which will give "add_key: Invalid argument" without this fix.
>
> Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")

[ CC stable@vger.kernel.org ]

Your (follow-up) patch is now upstream.

https://git.kernel.org/linus/acc657692aed438e9931438f8c923b2b107aebf9

This misses CC: Stable Tag as suggested by Linus.

Looks like linux-6.1.y and linux-6.6.y needs it, too.

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.6.11&id=da89365158f6f656b28bcdbcbbe9eaf97c63c474
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.1.72&id=079eefaecfd7bbb8fcc30eccb0dfdf50c91f1805

BG,
-Sedat-

> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Link: https://lore.kernel.org/r/ZZ4fyY4r3rqgZL+4@xpf.sh.intel.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Edward Adam Davis <eadavis@qq.com>
> cc: Linus Torvalds <torvalds@linux-foundation.org>
> cc: Simon Horman <horms@kernel.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jeffrey E Altman <jaltman@auristor.com>
> Cc: Wang Lei <wang840925@gmail.com>
> Cc: Jeff Layton <jlayton@redhat.com>
> Cc: Steve French <sfrench@us.ibm.com>
> Cc: Marc Dionne <marc.dionne@auristor.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/dns_resolver/dns_key.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
> index f18ca02aa95a..c42ddd85ff1f 100644
> --- a/net/dns_resolver/dns_key.c
> +++ b/net/dns_resolver/dns_key.c
> @@ -104,7 +104,7 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
>                 const struct dns_server_list_v1_header *v1;
>
>                 /* It may be a server list. */
> -               if (datalen <= sizeof(*v1))
> +               if (datalen < sizeof(*v1))
>                         return -EINVAL;
>
>                 v1 = (const struct dns_server_list_v1_header *)data;
>
>
Simon Horman Jan. 11, 2024, 1:19 p.m. UTC | #2
On Wed, Jan 10, 2024 at 09:11:40PM +0000, David Howells wrote:
>     
> Fix the size check added to dns_resolver_preparse() for the V1 server-list
> header so that it doesn't give EINVAL if the size supplied is the same as
> the size of the header struct (which should be valid).
> 
> This can be tested with:
> 
>         echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p
> 
> which will give "add_key: Invalid argument" without this fix.
> 
> Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Link: https://lore.kernel.org/r/ZZ4fyY4r3rqgZL+4@xpf.sh.intel.com/
> Signed-off-by: David Howells <dhowells@redhat.com>

Reviewed-by: Simon Horman <horms@kernel.org>
Jarkko Sakkinen Jan. 13, 2024, 8:40 p.m. UTC | #3
On Wed Jan 10, 2024 at 11:11 PM EET, David Howells wrote:
>     
> Fix the size check added to dns_resolver_preparse() for the V1 server-list
> header so that it doesn't give EINVAL if the size supplied is the same as
> the size of the header struct (which should be valid).
>
> This can be tested with:
>
>         echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p
>
> which will give "add_key: Invalid argument" without this fix.
>
> Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Link: https://lore.kernel.org/r/ZZ4fyY4r3rqgZL+4@xpf.sh.intel.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Edward Adam Davis <eadavis@qq.com>
> cc: Linus Torvalds <torvalds@linux-foundation.org>
> cc: Simon Horman <horms@kernel.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jeffrey E Altman <jaltman@auristor.com>
> Cc: Wang Lei <wang840925@gmail.com>
> Cc: Jeff Layton <jlayton@redhat.com>
> Cc: Steve French <sfrench@us.ibm.com>
> Cc: Marc Dionne <marc.dionne@auristor.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/dns_resolver/dns_key.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
> index f18ca02aa95a..c42ddd85ff1f 100644
> --- a/net/dns_resolver/dns_key.c
> +++ b/net/dns_resolver/dns_key.c
> @@ -104,7 +104,7 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
>  		const struct dns_server_list_v1_header *v1;
>  
>  		/* It may be a server list. */
> -		if (datalen <= sizeof(*v1))
> +		if (datalen < sizeof(*v1))
>  			return -EINVAL;
>  
>  		v1 = (const struct dns_server_list_v1_header *)data;

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko
Sedat Dilek Jan. 22, 2024, 11:01 a.m. UTC | #4
On Mon, Jan 22, 2024 at 8:33 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> From: Sedat Dilek <sedat.dilek@gmail.com>
>
> On Wed, Jan 10, 2024 at 10:12 PM David Howells <dhowells@redhat.com> wrote:
> >
> >
> > Fix the size check added to dns_resolver_preparse() for the V1 server-list
> > header so that it doesn't give EINVAL if the size supplied is the same as
> > the size of the header struct (which should be valid).
> >
> > This can be tested with:
> >
> >         echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p
> >
> > which will give "add_key: Invalid argument" without this fix.
> >
> > Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")
>
> [ CC stable@vger.kernel.org ]
>
> Your (follow-up) patch is now upstream.
>
> https://git.kernel.org/linus/acc657692aed438e9931438f8c923b2b107aebf9
>
> This misses CC: Stable Tag as suggested by Linus.
>
> Looks like linux-6.1.y and linux-6.6.y needs it, too.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.6.11&id=da89365158f6f656b28bcdbcbbe9eaf97c63c474
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.1.72&id=079eefaecfd7bbb8fcc30eccb0dfdf50c91f1805
>
> BG,
> -Sedat-
>
> Hi Greg, Sasa,
>
> could you please add this also to linux-6.1.y and linux-6.6.y?  (Easily
> applicable to both, needed for both.) Or is there any reason why it's not
> being added?
>

Great!

I forgot to CC Greg and Sasha directly.

Thanks.

BG,
-Sedat-

> Kind regards,
> Petr
>
> > Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> > Link: https://lore.kernel.org/r/ZZ4fyY4r3rqgZL+4@xpf.sh.intel.com/
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: Edward Adam Davis <eadavis@qq.com>
> > cc: Linus Torvalds <torvalds@linux-foundation.org>
> > cc: Simon Horman <horms@kernel.org>
> > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > Cc: Jeffrey E Altman <jaltman@auristor.com>
> > Cc: Wang Lei <wang840925@gmail.com>
> > Cc: Jeff Layton <jlayton@redhat.com>
> > Cc: Steve French <sfrench@us.ibm.com>
> > Cc: Marc Dionne <marc.dionne@auristor.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > ---
> >  net/dns_resolver/dns_key.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
> > index f18ca02aa95a..c42ddd85ff1f 100644
> > --- a/net/dns_resolver/dns_key.c
> > +++ b/net/dns_resolver/dns_key.c
> > @@ -104,7 +104,7 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
> >                 const struct dns_server_list_v1_header *v1;
> >
> >                 /* It may be a server list. */
> > -               if (datalen <= sizeof(*v1))
> > +               if (datalen < sizeof(*v1))
> >                         return -EINVAL;
> >
> >                 v1 = (const struct dns_server_list_v1_header *)data;
> >
> >
>
Sedat Dilek Jan. 22, 2024, 11:31 a.m. UTC | #5
On Mon, Jan 22, 2024 at 12:01 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Jan 22, 2024 at 8:33 AM Petr Vorel <pvorel@suse.cz> wrote:
> >
> > From: Sedat Dilek <sedat.dilek@gmail.com>
> >
> > On Wed, Jan 10, 2024 at 10:12 PM David Howells <dhowells@redhat.com> wrote:
> > >
> > >
> > > Fix the size check added to dns_resolver_preparse() for the V1 server-list
> > > header so that it doesn't give EINVAL if the size supplied is the same as
> > > the size of the header struct (which should be valid).
> > >
> > > This can be tested with:
> > >
> > >         echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p
> > >
> > > which will give "add_key: Invalid argument" without this fix.
> > >
> > > Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")
> >
> > [ CC stable@vger.kernel.org ]
> >
> > Your (follow-up) patch is now upstream.
> >
> > https://git.kernel.org/linus/acc657692aed438e9931438f8c923b2b107aebf9
> >
> > This misses CC: Stable Tag as suggested by Linus.
> >
> > Looks like linux-6.1.y and linux-6.6.y needs it, too.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.6.11&id=da89365158f6f656b28bcdbcbbe9eaf97c63c474
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.1.72&id=079eefaecfd7bbb8fcc30eccb0dfdf50c91f1805
> >
> > BG,
> > -Sedat-
> >
> > Hi Greg, Sasa,
> >
> > could you please add this also to linux-6.1.y and linux-6.6.y?  (Easily
> > applicable to both, needed for both.) Or is there any reason why it's not
> > being added?
> >
>
> Great!
>
> I forgot to CC Greg and Sasha directly.
>
> Thanks.
>

Addendum:

Linus says:
"
Bah. Obvious fix is obvious.

Mind sending it as a proper patch with sign-off etc, and we'll get
this fixed and marked for stable.
"

https://lore.kernel.org/all/CAHk-=wiyG8BKKZmU7CDHC8+rmvBndrqNSgLV6LtuqN8W_gL3hA@mail.gmail.com/

-Sedat-

> BG,
> -Sedat-
>
> > Kind regards,
> > Petr
> >
> > > Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> > > Link: https://lore.kernel.org/r/ZZ4fyY4r3rqgZL+4@xpf.sh.intel.com/
> > > Signed-off-by: David Howells <dhowells@redhat.com>
> > > cc: Edward Adam Davis <eadavis@qq.com>
> > > cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > cc: Simon Horman <horms@kernel.org>
> > > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > > Cc: Jeffrey E Altman <jaltman@auristor.com>
> > > Cc: Wang Lei <wang840925@gmail.com>
> > > Cc: Jeff Layton <jlayton@redhat.com>
> > > Cc: Steve French <sfrench@us.ibm.com>
> > > Cc: Marc Dionne <marc.dionne@auristor.com>
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Jakub Kicinski <kuba@kernel.org>
> > > Cc: Paolo Abeni <pabeni@redhat.com>
> > > ---
> > >  net/dns_resolver/dns_key.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
> > > index f18ca02aa95a..c42ddd85ff1f 100644
> > > --- a/net/dns_resolver/dns_key.c
> > > +++ b/net/dns_resolver/dns_key.c
> > > @@ -104,7 +104,7 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
> > >                 const struct dns_server_list_v1_header *v1;
> > >
> > >                 /* It may be a server list. */
> > > -               if (datalen <= sizeof(*v1))
> > > +               if (datalen < sizeof(*v1))
> > >                         return -EINVAL;
> > >
> > >                 v1 = (const struct dns_server_list_v1_header *)data;
> > >
> > >
> >
Greg KH Jan. 22, 2024, 3:02 p.m. UTC | #6
On Mon, Jan 22, 2024 at 08:32:20AM +0100, Petr Vorel wrote:
> From: Sedat Dilek <sedat.dilek@gmail.com>
> 
> On Wed, Jan 10, 2024 at 10:12 PM David Howells <dhowells@redhat.com> wrote:
> >
> >
> > Fix the size check added to dns_resolver_preparse() for the V1 server-list
> > header so that it doesn't give EINVAL if the size supplied is the same as
> > the size of the header struct (which should be valid).
> >
> > This can be tested with:
> >
> >         echo -n -e '\0\0\01\xff\0\0' | keyctl padd dns_resolver desc @p
> >
> > which will give "add_key: Invalid argument" without this fix.
> >
> > Fixes: 1997b3cb4217 ("keys, dns: Fix missing size check of V1 server-list header")
> 
> [ CC stable@vger.kernel.org ]
> 
> Your (follow-up) patch is now upstream.
> 
> https://git.kernel.org/linus/acc657692aed438e9931438f8c923b2b107aebf9
> 
> This misses CC: Stable Tag as suggested by Linus.
> 
> Looks like linux-6.1.y and linux-6.6.y needs it, too.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.6.11&id=da89365158f6f656b28bcdbcbbe9eaf97c63c474
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.1.72&id=079eefaecfd7bbb8fcc30eccb0dfdf50c91f1805

And 5.10.y and 5.15.y.  Now queued up, thanks.

greg k-h
diff mbox series

Patch

diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index f18ca02aa95a..c42ddd85ff1f 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -104,7 +104,7 @@  dns_resolver_preparse(struct key_preparsed_payload *prep)
 		const struct dns_server_list_v1_header *v1;
 
 		/* It may be a server list. */
-		if (datalen <= sizeof(*v1))
+		if (datalen < sizeof(*v1))
 			return -EINVAL;
 
 		v1 = (const struct dns_server_list_v1_header *)data;