All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: keyrings@vger.kernel.org, David Howells <dhowells@redhat.com>
Cc: netdev@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH] KEYS: DNS: limit the length of option strings
Date: Wed, 28 Feb 2018 01:49:05 +0000	[thread overview]
Message-ID: <20180228014905.68153-1-ebiggers3@gmail.com> (raw)

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.

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]")
Cc: <stable@vger.kernel.org> # v2.6.36+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 net/dns_resolver/dns_key.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index e1d4d898a007..7c0aae2e512d 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("Invalid option length (%d) for dns_resolver key\n",
+					opt_len);
 				return -EINVAL;
 			}
 
-- 
2.16.2.395.g2e18187dfd-goog


WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers3@gmail.com>
To: keyrings@vger.kernel.org, David Howells <dhowells@redhat.com>
Cc: netdev@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH] KEYS: DNS: limit the length of option strings
Date: Tue, 27 Feb 2018 17:49:05 -0800	[thread overview]
Message-ID: <20180228014905.68153-1-ebiggers3@gmail.com> (raw)

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.

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]")
Cc: <stable@vger.kernel.org> # v2.6.36+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 net/dns_resolver/dns_key.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index e1d4d898a007..7c0aae2e512d 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("Invalid option length (%d) for dns_resolver key\n",
+					opt_len);
 				return -EINVAL;
 			}
 
-- 
2.16.2.395.g2e18187dfd-goog

             reply	other threads:[~2018-02-28  1:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-28  1:49 Eric Biggers [this message]
2018-02-28  1:49 ` [PATCH] KEYS: DNS: limit the length of option strings Eric Biggers
2018-02-28  2:34 ` Eric Dumazet
2018-02-28  2:34   ` Eric Dumazet
2018-02-28 19:04   ` Eric Biggers
2018-02-28 19:04     ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180228014905.68153-1-ebiggers3@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=ebiggers@google.com \
    --cc=keyrings@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.