From patchwork Wed Jul 11 17:46:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 942614 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="hvg3KWlH"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41Qmhx6KGZz9s0n for ; Thu, 12 Jul 2018 03:46:41 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389053AbeGKRwD (ORCPT ); Wed, 11 Jul 2018 13:52:03 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:40168 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389026AbeGKRwD (ORCPT ); Wed, 11 Jul 2018 13:52:03 -0400 Received: by mail-pf0-f194.google.com with SMTP id e13-v6so5507640pff.7; Wed, 11 Jul 2018 10:46:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=9rbStQNZUN5LhdvZR0EOWRi3dU/r5Ki8Oj61cA8htYE=; b=hvg3KWlHuyhW0nUpuhuEPOVzrMSnLgCkCf2MsuuOnIIiqNQacmBUcIGqfLYdq1jCMi KOHlJsPUG6l+XIYOFXfG90t/0JVQcUjf1q1NtiXhRxXqO9/IEvsIZLxc9uvzF5dAUc7M 8Btn7j6P0vcdSkUaAcANWAI6G/G059yt/Om3UPmjW1Gl2yfSXr6CQFf30O+kJeh4ubWB wf0Dlq0JEZUHTvuHuAhO1esS9kkH1JI1J9SNCySUSB93sKO48Mx06Zw5Fch/PP83u7fC w2r7zs44GrALStkdm3haxn52WIMR9qIEZwLNxPOb7QLVSvgZRanZKstSS7WvhA0bL0Dw yntQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=9rbStQNZUN5LhdvZR0EOWRi3dU/r5Ki8Oj61cA8htYE=; b=p6QxFEAL9Iy37KjXopgjDIakUi1GGYNr8Q0VnLkNHICJ7zO5U4263mk+080goWGxys hde1Q4msVbXtLkaXtsPnwIiOtTZrwFIDxNW8WYDT7TABy3+OY7o1g70g2I36dCKd10mf lmYxkmJfRe+24GiCNaMbXUx6OeN6cG4GXk+i1uU/dG6PBTfCgcztUC8g4TmIMw9cQN8m lI4XXV7j1eNEK4cVP9aYAE+2XUdNHks1nckHc13kxVmB10F+0EI1F2fz/2V0kYn8a5Uq HJtgDlO5z8q8i2smPLnOECR4TeIYJXDawfvBXiHv9QgO+tw+nswTN0A1jPYtuyDyDN8V Fe9A== X-Gm-Message-State: APt69E21WyyXD0hIoGN4+Up0wTeM6WJ9/0GK3u8UrTa1k9EVKR9ZHSno 55anlN2tUy/z5JMKWcRMA9Oknb84 X-Google-Smtp-Source: AAOMgpfyhZDvL/vkGXSSJ0OT+NRmrWBeY24QMyNrKIY9zxUJYArxdqT3dz7p+3TrPbIF6Hu1m5h2ng== X-Received: by 2002:a62:c60e:: with SMTP id m14-v6mr20172984pfg.40.1531331198301; Wed, 11 Jul 2018 10:46:38 -0700 (PDT) Received: from ebiggers-linuxstation.kir.corp.google.com ([2620:15c:17:3:dc28:5c82:b905:e8a8]) by smtp.gmail.com with ESMTPSA id w81-v6sm36717785pfk.92.2018.07.11.10.46.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Jul 2018 10:46:37 -0700 (PDT) From: Eric Biggers To: netdev@vger.kernel.org, "David S . Miller" Cc: keyrings@vger.kernel.org, David Howells , Wang Lei , Eric Biggers Subject: [PATCH net v2] KEYS: DNS: fix parsing multiple options Date: Wed, 11 Jul 2018 10:46:29 -0700 Message-Id: <20180711174629.2700-1-ebiggers3@gmail.com> X-Mailer: git-send-email 2.18.0.203.gfac676dfb9-goog Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Eric Biggers 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 Fixes: 4a2d789267e0 ("DNS: If the DNS server returns an error, allow that to be cached [ver #2]") Signed-off-by: Eric Biggers --- Changed since v1: - Also fix parsing the option values, not just option names. net/dns_resolver/dns_key.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c index 40c851693f77..0c9478b91fa5 100644 --- a/net/dns_resolver/dns_key.c +++ b/net/dns_resolver/dns_key.c @@ -86,35 +86,39 @@ dns_resolver_preparse(struct key_preparsed_payload *prep) opt++; kdebug("options: '%s'", opt); do { + int opt_len, opt_nlen; const char *eq; - int opt_len, opt_nlen, opt_vlen, tmp; + char optval[128]; next_opt = memchr(opt, '#', end - opt) ?: end; opt_len = next_opt - opt; - if (opt_len <= 0 || opt_len > 128) { + if (opt_len <= 0 || opt_len > sizeof(optval)) { pr_warn_ratelimited("Invalid option length (%d) for dns_resolver key\n", opt_len); return -EINVAL; } - eq = memchr(opt, '=', opt_len) ?: end; - opt_nlen = eq - opt; - eq++; - opt_vlen = next_opt - eq; /* will be -1 if no value */ + eq = memchr(opt, '=', opt_len); + if (eq) { + opt_nlen = eq - opt; + eq++; + memcpy(optval, eq, next_opt - eq); + optval[next_opt - eq] = '\0'; + } else { + opt_nlen = opt_len; + optval[0] = '\0'; + } - tmp = opt_vlen >= 0 ? opt_vlen : 0; - kdebug("option '%*.*s' val '%*.*s'", - opt_nlen, opt_nlen, opt, tmp, tmp, eq); + kdebug("option '%*.*s' val '%s'", + opt_nlen, opt_nlen, opt, optval); /* see if it's an error number representing a DNS error * that's to be recorded as the result in this key */ if (opt_nlen == sizeof(DNS_ERRORNO_OPTION) - 1 && memcmp(opt, DNS_ERRORNO_OPTION, opt_nlen) == 0) { kdebug("dns error number option"); - if (opt_vlen <= 0) - goto bad_option_value; - ret = kstrtoul(eq, 10, &derrno); + ret = kstrtoul(optval, 10, &derrno); if (ret < 0) goto bad_option_value;