| Message ID | 20180416212922.233194-1-ebiggers3@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [RESEND,net-next,v2] KEYS: DNS: limit the length of option strings | expand |
From: Eric Biggers <ebiggers3@gmail.com> Date: Mon, 16 Apr 2018 14:29:22 -0700 > From: Eric Biggers <ebiggers@google.com> > > Adding a dns_resolver key whose payload contains a very long option name > resulted in that string being printed in full. This hit the WARN_ONCE() > in set_precision() during the printk(), because printk() only supports a > precision of up to 32767 bytes: > > precision 1000000 too large > WARNING: CPU: 0 PID: 752 at lib/vsprintf.c:2189 vsnprintf+0x4bc/0x5b0 > > Fix it by limiting option strings (combined name + value) to a much more > reasonable 128 bytes. The exact limit is arbitrary, but currently the > only recognized option is formatted as "dnserror=%lu" which fits well > within this limit. > > Also ratelimit the printks. > > Reproducer: > > perl -e 'print "#", "A" x 1000000, "\x00"' | keyctl padd dns_resolver desc @s > > This bug was found using syzkaller. > > Reported-by: Mark Rutland <mark.rutland@arm.com> > Fixes: 4a2d789267e0 ("DNS: If the DNS server returns an error, allow that to be cached [ver #2]") > Signed-off-by: Eric Biggers <ebiggers@google.com> Applied, thanks.
On Tue, Apr 17, 2018 at 01:43:16PM -0400, David Miller wrote: > From: Eric Biggers <ebiggers3@gmail.com> > Date: Mon, 16 Apr 2018 14:29:22 -0700 > > > From: Eric Biggers <ebiggers@google.com> > > > > Adding a dns_resolver key whose payload contains a very long option name > > resulted in that string being printed in full. This hit the WARN_ONCE() > > in set_precision() during the printk(), because printk() only supports a > > precision of up to 32767 bytes: > > > > precision 1000000 too large > > WARNING: CPU: 0 PID: 752 at lib/vsprintf.c:2189 vsnprintf+0x4bc/0x5b0 > > > > Fix it by limiting option strings (combined name + value) to a much more > > reasonable 128 bytes. The exact limit is arbitrary, but currently the > > only recognized option is formatted as "dnserror=%lu" which fits well > > within this limit. > > > > Also ratelimit the printks. > > > > Reproducer: > > > > perl -e 'print "#", "A" x 1000000, "\x00"' | keyctl padd dns_resolver desc @s > > > > This bug was found using syzkaller. > > > > Reported-by: Mark Rutland <mark.rutland@arm.com> > > Fixes: 4a2d789267e0 ("DNS: If the DNS server returns an error, allow that to be cached [ver #2]") > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > Applied, thanks. Can you queue this up for stable too? syzbot has been hitting this on older kernel versions. Eric
From: Eric Biggers <ebiggers3@gmail.com> Date: Tue, 17 Apr 2018 11:23:40 -0700 > Can you queue this up for stable too? syzbot has been hitting this on older > kernel versions. If you want a patch bound for stable, it must show up in Linus's tree first which means you should target 'net' rather than 'net-next'.
On Tue, Apr 17, 2018 at 02:24:37PM -0400, David Miller wrote: > From: Eric Biggers <ebiggers3@gmail.com> > Date: Tue, 17 Apr 2018 11:23:40 -0700 > > > Can you queue this up for stable too? syzbot has been hitting this on older > > kernel versions. > > If you want a patch bound for stable, it must show up in Linus's tree > first which means you should target 'net' rather than 'net-next'. Okay, can you move the patch there, or do I need to resend, or is it too late already? It's a clean cherry-pick. Sorry, I'm not too familiar with the quirks of net and netdev -- most other maintainers do things differently. Thanks, Eric
From: Eric Biggers <ebiggers3@gmail.com> Date: Tue, 17 Apr 2018 11:37:36 -0700 > On Tue, Apr 17, 2018 at 02:24:37PM -0400, David Miller wrote: >> From: Eric Biggers <ebiggers3@gmail.com> >> Date: Tue, 17 Apr 2018 11:23:40 -0700 >> >> > Can you queue this up for stable too? syzbot has been hitting this on older >> > kernel versions. >> >> If you want a patch bound for stable, it must show up in Linus's tree >> first which means you should target 'net' rather than 'net-next'. > > Okay, can you move the patch there, or do I need to resend, or is it too late > already? It's a clean cherry-pick. Sorry, I'm not too familiar with the quirks > of net and netdev -- most other maintainers do things differently. It's already in net-next, so submit another copy based on net.
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c index 8396705deffc..40c851693f77 100644 --- a/net/dns_resolver/dns_key.c +++ b/net/dns_resolver/dns_key.c @@ -91,9 +91,9 @@ dns_resolver_preparse(struct key_preparsed_payload *prep) next_opt = memchr(opt, '#', end - opt) ?: end; opt_len = next_opt - opt; - if (!opt_len) { - printk(KERN_WARNING - "Empty option to dns_resolver key\n"); + if (opt_len <= 0 || opt_len > 128) { + pr_warn_ratelimited("Invalid option length (%d) for dns_resolver key\n", + opt_len); return -EINVAL; } @@ -127,10 +127,8 @@ dns_resolver_preparse(struct key_preparsed_payload *prep) } bad_option_value: - printk(KERN_WARNING - "Option '%*.*s' to dns_resolver key:" - " bad/missing value\n", - opt_nlen, opt_nlen, opt); + pr_warn_ratelimited("Option '%*.*s' to dns_resolver key: bad/missing value\n", + opt_nlen, opt_nlen, opt); return -EINVAL; } while (opt = next_opt + 1, opt < end); }