From patchwork Tue Dec 5 16:41:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Ahern X-Patchwork-Id: 10093355 X-Patchwork-Delegate: johannes@sipsolutions.net Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7775160329 for ; Tue, 5 Dec 2017 16:41:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 63848286C2 for ; Tue, 5 Dec 2017 16:41:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 61CDC28756; Tue, 5 Dec 2017 16:41:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 682D628C8D for ; Tue, 5 Dec 2017 16:41:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752548AbdLEQlZ (ORCPT ); Tue, 5 Dec 2017 11:41:25 -0500 Received: from mail-pg0-f49.google.com ([74.125.83.49]:42498 "EHLO mail-pg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752065AbdLEQlX (ORCPT ); Tue, 5 Dec 2017 11:41:23 -0500 Received: by mail-pg0-f49.google.com with SMTP id e14so510414pgr.9; Tue, 05 Dec 2017 08:41:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=301kY7wzlNu4O6GlkyavFAwRgkzMANcrfj/hCq4tRRA=; b=AnofahYZaj5lXZE0roK+uolftVGZpSxC4TcAvGKriz2EJuOsCIhzVSwVpSjZPtVOk6 wxm0lZz50PwGOJqH6FGBOROE1WPF3A0BaYD1sSpNBrmYTN9JrUUbjkhckcJML0BmYyUW zJ9+3XU9X//nZFQxFxSNr8nNvTvD/6ECVxSqQs7nkqIYqYvMuwkde1fyGNOmU3fEemSR LGNF/gchg2MQjabEbzSLftnBHx7/bA5AnV2lYulp1JZvyMRN5sNu16nyUsv7/VgUFbJE +WHQ9104PPiAUlgZbPtaF8YyX4qi2tVhLLD4mYXsknQ8tXw44LpbEnrDJCLdObkOr8Jf HuSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=301kY7wzlNu4O6GlkyavFAwRgkzMANcrfj/hCq4tRRA=; b=YEjmMWtLPVXCnl5x9DHBwL15l4G83dqY/8vD5o+HLZ6PJbUZ+tUrle/qZ2vxVeL8vW uX/UNXdJSXhZ/wvLbULqi+By3zjx++GRMU7CtVP8GXcAjrBc04IjpXzvENBZpKV+Jx91 DI4k1xchYjqdEksQHbpEj390UJIMdPDhLbI9hIIgkdItcZY24c9b5U4meMWWvHcPE8NO +QSf5nrQuxgzZDMzg+g4rZgw2r8B1KDtA0t3jNpoiFtiv1cQtfXJio2ue8I+ZE5+/3w4 94tB53zSa3PNvzQK7eivUFsQKC1tKouBof/Tx0J7d0B5d5RTbhSzaXPDoQrdi8GH4X7Y Ti+g== X-Gm-Message-State: AJaThX4bhiL1z+tfYvz+xTZ01N0HmGq+j/fQc+MRYP7KJPM/qHUHCDtv 95xEyzD12as01+rtnJT3zAlJnw== X-Google-Smtp-Source: AGs4zMaw0B0BknTP1oZP8H/clNnTeH6SxIVIf7pNyoh2LwaVE6+AqTze87MFcnO55Z1zyHRj+4BeAQ== X-Received: by 10.101.90.8 with SMTP id y8mr18069284pgs.21.1512492082888; Tue, 05 Dec 2017 08:41:22 -0800 (PST) Received: from dsa-mb.local ([2601:282:800:7292:e5f7:1722:5a01:5eaf]) by smtp.googlemail.com with ESMTPSA id w73sm803045pfd.86.2017.12.05.08.41.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 Dec 2017 08:41:22 -0800 (PST) Subject: Re: [PATCH net 1/2] netlink: add NLA_U8_BUGGY attribute type To: Johannes Berg , David Miller Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org, j@w1.fi References: <20171202202332.10205-1-johannes@sipsolutions.net> <20171205.113145.172521292247335321.davem@davemloft.net> <1512491661.26976.19.camel@sipsolutions.net> From: David Ahern Message-ID: Date: Tue, 5 Dec 2017 09:41:21 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <1512491661.26976.19.camel@sipsolutions.net> Content-Language: en-US Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 12/5/17 9:34 AM, Johannes Berg wrote: > On Tue, 2017-12-05 at 11:31 -0500, David Miller wrote: >> >>> We could try to fix up the big endian problem here, but we >>> don't know *how* userspace misbehaved - if using nla_put_u32 >>> then we could, but we also found a debug tool (which we'll >>> ignore for the purposes of this regression) that was putting >>> the padding into the length. > >> We're stuck with this thing forever... I'd like to consider other >> options. >> >> I've seen this problem at least one time before, therefore I >> suggest when we see a U8 attribute with a U32's length: >> >> 1) We access it as a u32, this takes care of all endianness >> issues. > > Possible, but as I said above, I've seen at least one tool (a debug > only script) now that will actually emit a U8 followed by 3 bytes of > padding to make it netlink-aligned, but set the length to 4. That would > be broken by making this change. > > I'm not saying this is bad - but there are different levels of > compatibility and I'd probably go for "bug compatibility" here rather > than "fix-it-up compatibility". > > Your call, ultimately - I've already fixed the tool I had found :-) > >> 2) We emit a warning so that the app gets fixes. > The attached is my proposal. Basically, allow it the length mismatch but print a warning. This restores previous behavior and tells users of bad commands. From 29a504c8de553c17d4aafd5a2cb9384a28672fe4 Mon Sep 17 00:00:00 2001 From: David Ahern Date: Sun, 3 Dec 2017 07:22:20 -0800 Subject: [PATCH] netlink: Relax attr validation for fixed length types Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types") requires attributes using types NLA_U* and NLA_S* to have an exact length. This change is exposing bugs in various userspace commands that are sending attributes with an invalid length (e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While the commands are clearly broken and need to be fixed, users are arguing that the sudden change in enforcement is breaking older commands on newer kernels for use cases that otherwise "worked". Relax the validation to print a warning mesage similar to what is done for messages containing extra bytes after parsing. Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types") Signed-off-by: David Ahern --- lib/nlattr.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/nlattr.c b/lib/nlattr.c index 8bf78b4b78f0..6122662906c8 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -28,8 +28,16 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = { }; static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = { + [NLA_U8] = sizeof(u8), + [NLA_U16] = sizeof(u16), + [NLA_U32] = sizeof(u32), + [NLA_U64] = sizeof(u64), [NLA_MSECS] = sizeof(u64), [NLA_NESTED] = NLA_HDRLEN, + [NLA_S8] = sizeof(s8), + [NLA_S16] = sizeof(s16), + [NLA_S32] = sizeof(s32), + [NLA_S64] = sizeof(s64), }; static int validate_nla_bitfield32(const struct nlattr *nla, @@ -70,10 +78,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype, BUG_ON(pt->type > NLA_TYPE_MAX); /* for data types NLA_U* and NLA_S* require exact length */ - if (nla_attr_len[pt->type]) { - if (attrlen != nla_attr_len[pt->type]) - return -ERANGE; - return 0; + if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) { + pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n", + current->comm, type); } switch (pt->type) {