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