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

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

[David Howells]

    
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;

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Sedat Dilek]

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;
>
>

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Simon Horman]

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>

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Jarkko Sakkinen]

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

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Petr Vorel]

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?

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;
>
>

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Sedat Dilek]

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;
> >
> >
>

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Sedat Dilek]

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;
> > >
> > >
> >

Re: [PATCH] keys, dns: Fix size check of V1 server-list header

[Greg KH]

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