Skip to content

Commit 9afe57f

Browse files
ebiggersZhengShunQian
authored andcommitted
KEYS: DNS: fix parsing multiple options
commit c604cb7 upstream. My recent fix for dns_resolver_preparse() printing very long strings was incomplete, as shown by syzbot which still managed to hit the WARN_ONCE() in set_precision() by adding a crafted "dns_resolver" key: precision 50001 too large WARNING: CPU: 7 PID: 864 at lib/vsprintf.c:2164 vsnprintf+0x48a/0x5a0 The bug this time isn't just a printing bug, but also a logical error when multiple options ("#"-separated strings) are given in the key payload. Specifically, when separating an option string into name and value, if there is no value then the name is incorrectly considered to end at the end of the key payload, rather than the end of the current option. This bypasses validation of the option length, and also means that specifying multiple options is broken -- which presumably has gone unnoticed as there is currently only one valid option anyway. A similar problem also applied to option values, as the kstrtoul() when parsing the "dnserror" option will read past the end of the current option and into the next option. Fix these bugs by correctly computing the length of the option name and by copying the option value, null-terminated, into a temporary buffer. Reproducer for the WARN_ONCE() that syzbot hit: perl -e 'print "#A#", "\0" x 50000' | keyctl padd dns_resolver desc @s Reproducer for "dnserror" option being parsed incorrectly (expected behavior is to fail when seeing the unknown option "foo", actual behavior was to read the dnserror value as "1#foo" and fail there): perl -e 'print "#dnserror=1#foo\0"' | keyctl padd dns_resolver desc @s Reported-by: syzbot <syzkaller@googlegroups.com> Fixes: 4a2d789 ("DNS: If the DNS server returns an error, allow that to be cached [ver #2]") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent dbf307e commit 9afe57f

File tree

1 file changed

+16
-12
lines changed

1 file changed

+16
-12
lines changed

net/dns_resolver/dns_key.c

+16-12
Original file line numberDiff line numberDiff line change
@@ -87,35 +87,39 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
8787
opt++;
8888
kdebug("options: '%s'", opt);
8989
do {
90+
int opt_len, opt_nlen;
9091
const char *eq;
91-
int opt_len, opt_nlen, opt_vlen, tmp;
92+
char optval[128];
9293

9394
next_opt = memchr(opt, '#', end - opt) ?: end;
9495
opt_len = next_opt - opt;
95-
if (opt_len <= 0 || opt_len > 128) {
96+
if (opt_len <= 0 || opt_len > sizeof(optval)) {
9697
pr_warn_ratelimited("Invalid option length (%d) for dns_resolver key\n",
9798
opt_len);
9899
return -EINVAL;
99100
}
100101

101-
eq = memchr(opt, '=', opt_len) ?: end;
102-
opt_nlen = eq - opt;
103-
eq++;
104-
opt_vlen = next_opt - eq; /* will be -1 if no value */
102+
eq = memchr(opt, '=', opt_len);
103+
if (eq) {
104+
opt_nlen = eq - opt;
105+
eq++;
106+
memcpy(optval, eq, next_opt - eq);
107+
optval[next_opt - eq] = '\0';
108+
} else {
109+
opt_nlen = opt_len;
110+
optval[0] = '\0';
111+
}
105112

106-
tmp = opt_vlen >= 0 ? opt_vlen : 0;
107-
kdebug("option '%*.*s' val '%*.*s'",
108-
opt_nlen, opt_nlen, opt, tmp, tmp, eq);
113+
kdebug("option '%*.*s' val '%s'",
114+
opt_nlen, opt_nlen, opt, optval);
109115

110116
/* see if it's an error number representing a DNS error
111117
* that's to be recorded as the result in this key */
112118
if (opt_nlen == sizeof(DNS_ERRORNO_OPTION) - 1 &&
113119
memcmp(opt, DNS_ERRORNO_OPTION, opt_nlen) == 0) {
114120
kdebug("dns error number option");
115-
if (opt_vlen <= 0)
116-
goto bad_option_value;
117121

118-
ret = kstrtoul(eq, 10, &derrno);
122+
ret = kstrtoul(optval, 10, &derrno);
119123
if (ret < 0)
120124
goto bad_option_value;
121125

0 commit comments

Comments
 (0)