From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13877C433E0 for ; Tue, 19 Jan 2021 19:45:18 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 815B323138 for ; Tue, 19 Jan 2021 19:45:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 815B323138 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A62E16B0005; Tue, 19 Jan 2021 14:45:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9EE7F6B0006; Tue, 19 Jan 2021 14:45:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8DBB86B0007; Tue, 19 Jan 2021 14:45:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0180.hostedemail.com [216.40.44.180]) by kanga.kvack.org (Postfix) with ESMTP id 69F436B0005 for ; Tue, 19 Jan 2021 14:45:16 -0500 (EST) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 2B4791EE6 for ; Tue, 19 Jan 2021 19:45:16 +0000 (UTC) X-FDA: 77723553432.26.woman18_2b11eec27554 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin26.hostedemail.com (Postfix) with ESMTP id 00E9A1804B669 for ; Tue, 19 Jan 2021 19:45:15 +0000 (UTC) X-HE-Tag: woman18_2b11eec27554 X-Filterd-Recvd-Size: 7059 Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) by imf39.hostedemail.com (Postfix) with ESMTP for ; Tue, 19 Jan 2021 19:45:15 +0000 (UTC) Received: by mail-pf1-f182.google.com with SMTP id w14so3887704pfi.2 for ; Tue, 19 Jan 2021 11:45:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=TZWPmeK6nXakTnptNSiDMwmmfuwKT59whEZJ3rHxaLw=; b=TaMSshX4B2n3vFKApyAfeBOtLDmP9J8gTr+lW7M5qmFsq1Iu5oIS2cAbdh+m+ygRMA SYAk2DeKQUExNPiEVdZWzlsy9VA8O1OnQYQx5IBoaogF2hXo5nselpRh0r0wc3iO0+hL HcNIHqDDFovEbhw27QUGrQZcFIZa8mMgo3gK0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=TZWPmeK6nXakTnptNSiDMwmmfuwKT59whEZJ3rHxaLw=; b=EpjTjKWu35xE0wLTUuUQUeZ7zVyqSKALAVrphf8zNwForO++gIXRgnyfdvCDClbPsI Easv/mYH+wIyc4D1WTN1MmmhONnqolEBP1jtP/NnTa7ANgjGnpJiHRwrnZDe81RLkiuM 0k6B9JxgH0hHvWJdmrElMTGco+KKiIkJQb6EpQXCZ+ZzqfU7wnGHcM1yttwMWjANAXKy hqhJQizzZwPrsiORcjwrzk6iugSZNf4WS80CJpVLcFz9WJ9IoBtZb1kfbSd+/F6Ha01A qFLAv9Gz0+M1dbSuKIXEVC3jfkfebWhsENtkttV65avGgnq2lzNQTQcG41mbUNzGumEJ amZg== X-Gm-Message-State: AOAM532YrySa6DJzV9CNS82MZDRgx334BLbL49i+RdNFNUSemYIzP+LU g8BEWwcs+qHO2iicE+18BGBnhg== X-Google-Smtp-Source: ABdhPJykeUe7VPA5lRIwyEFZ7G5haUHZJx1AiWjgDLdACfjZ80+WTKMRpIgdowymxogDkkcDmHUg7w== X-Received: by 2002:a63:5a08:: with SMTP id o8mr5915862pgb.118.1611085514157; Tue, 19 Jan 2021 11:45:14 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id 76sm2838040pfz.174.2021.01.19.11.45.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Jan 2021 11:45:13 -0800 (PST) Date: Tue, 19 Jan 2021 11:45:12 -0800 From: Kees Cook To: Matthew Wilcox Cc: Sergey Senozhatsky , Timur Tabi , Andrew Morton , linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, Petr Mladek , roman.fietze@magna.com, Steven Rostedt , John Ogness , linux-mm@kvack.org, Akinobu Mita Subject: Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps Message-ID: <202101191135.A78A570@keescook> References: <20210116220950.47078-1-timur@kernel.org> <20210118182635.GD2260413@casper.infradead.org> <20210119014725.GH2260413@casper.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210119014725.GH2260413@casper.infradead.org> Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jan 19, 2021 at 01:47:25AM +0000, Matthew Wilcox wrote: > On Tue, Jan 19, 2021 at 09:53:01AM +0900, Sergey Senozhatsky wrote: > > On (21/01/18 13:03), Timur Tabi wrote: > > > On 1/18/21 12:26 PM, Matthew Wilcox wrote: > > > > Don't make it easy. And don't make it look like they're doing > > > > something innocent. DUMP_PREFIX_SECURITY_HOLE would be OK > > > > by me. DUMP_PREFIX_LEAK_INFORMATION would work fine too. > > > > DUMP_PREFIX_MAKE_ATTACKERS_LIFE_EASY might be a bit too far. > > >=20 > > > It's already extremely easy to replace %p with %px in your own prin= tks, so I > > > don't really understand your argument. > >=20 > > I like the idea of a more radical name, e.g. DUMP_PREFIX_RAW_POINTERS= or > > something similar. > >=20 > > > Seriously, this patch should not be so contentious. If you want ha= shed > > > addresses, then nothing changes. If you need unhashed addresses wh= ile > > > debugging, then use DUMP_PREFIX_UNHASHED. Just like you can use %p= x in > > > printk. I never use %p in my printks, but then I never submit code= upstream > > > that prints addresses, hashed or unhashed. >=20 > I'm glad to hear you never make mistakes. I make lots of mistakes, so > I prefer them to be big, loud and obvious so they're easy for people > to spot. >=20 > > So maybe DUMP_PREFIX_UNHASHED can do the unhashed dump only when > > CONFIG_DEBUG_KERNEL=3Dy and fallback to DUMP_PREFIX_ADDRESS otherwise= ? >=20 > Distros enable CONFIG_DEBUG_KERNEL. If you want to add > CONFIG_DEBUG_LEAK_ADDRESSES, then that's great, and you won't even have > to change users, you can just change how %p behaves. Following Linus's guidance[1] on this kind of thing, I think the correct patch would be to actually _remove_ DUMP_PREFIX_ADDRESS entirely (or make the offset math be hash-based). There isn't a strong enough reason for it to exist in the first place: - If the hashed =E2=80=9C%p=E2=80=9D value is pointless, ask yourself whe= ther the pointer itself is important. Maybe it should be removed entirely? - If you really think the true pointer value is important, why is some system state or user privilege level considered =E2=80=9Cspecial=E2=80=9D= ? If you think you can justify it (in comments and commit log) well enough to stand up to Linus=E2=80=99s scrutiny, maybe you can use =E2=80=9C%px=E2=80=9D, a= long with making sure you have sensible permissions. - A toggle for =E2=80=9C%p=E2=80=9D hashing will not be accepted. How about this so the base address is hashed once, with the offset added to it for each line instead of each line having a "new" hash that makes no sense: diff --git a/lib/hexdump.c b/lib/hexdump.c index 9301578f98e8..20264828752d 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -242,12 +242,17 @@ void print_hex_dump(const char *level, const char *= prefix_str, int prefix_type, const void *buf, size_t len, bool ascii) { const u8 *ptr =3D buf; + const u8 *addr; int i, linelen, remaining =3D len; unsigned char linebuf[32 * 3 + 2 + 32 + 1]; =20 if (rowsize !=3D 16 && rowsize !=3D 32) rowsize =3D 16; =20 + if (prefix_type =3D=3D DUMP_PREFIX_ADDRESS && + ptr_to_hashval(ptr, &addr)) + addr =3D 0; + for (i =3D 0; i < len; i +=3D rowsize) { linelen =3D min(remaining, rowsize); remaining -=3D rowsize; @@ -258,7 +263,7 @@ void print_hex_dump(const char *level, const char *pr= efix_str, int prefix_type, switch (prefix_type) { case DUMP_PREFIX_ADDRESS: printk("%s%s%p: %s\n", - level, prefix_str, ptr + i, linebuf); + level, prefix_str, addr + i, linebuf); break; case DUMP_PREFIX_OFFSET: printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf); -Kees [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#p-form= at-specifier --=20 Kees Cook