From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751465AbdJSAAu (ORCPT ); Wed, 18 Oct 2017 20:00:50 -0400 Received: from mail-pf0-f174.google.com ([209.85.192.174]:45496 "EHLO mail-pf0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750942AbdJSAAt (ORCPT ); Wed, 18 Oct 2017 20:00:49 -0400 X-Google-Smtp-Source: AOwi7QAydGJc9PD8Pmp30L8u1ArvgIaqKBJni6UsRs7N04DIDh3gXBfRurrXFPXGKptljlB6uRpvTw== Date: Thu, 19 Oct 2017 09:00:44 +0900 From: Sergey Senozhatsky To: "Tobin C. Harding" Cc: Sergey Senozhatsky , kernel-hardening@lists.openwall.com, Linus Torvalds , Kees Cook , Paolo Bonzini , Tycho Andersen , "Roberts, William C" , Tejun Heo , Jordan Glover , Greg KH , Petr Mladek , Joe Perches , Ian Campbell , Sergey Senozhatsky , Catalin Marinas , Will Deacon , Steven Rostedt , Chris Fries , Dave Weinstein , Daniel Micay , Djalal Harouni , linux-kernel@vger.kernel.org Subject: Re: [PATCH v4] printk: hash addresses printed with %p Message-ID: <20171019000044.GA604@jagdpanzerIV> References: <1508300515-28824-1-git-send-email-me@tobin.cc> <20171018054431.GA597@jagdpanzerIV> <20171018060406.GA28753@eros> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171018060406.GA28753@eros> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (10/18/17 17:04), Tobin C. Harding wrote: [..] > > > +/* protects ptr_secret and have_key */ > > > +DEFINE_SPINLOCK(key_lock); > > > +static siphash_key_t ptr_secret __read_mostly; > > > +static atomic_t have_key = ATOMIC_INIT(0); > > > + > > > +static int initialize_ptr_secret(void) > > > +{ > > > + spin_lock(&key_lock); > > > + if (atomic_read(&have_key) == 1) > > > + goto unlock; > > > + > > > + get_random_bytes(&ptr_secret, sizeof(ptr_secret)); > > > + atomic_set(&have_key, 1); > > > + > > > +unlock: > > > + spin_unlock(&key_lock); > > > + return 0; > > > +} > > > > is this spinlock legal? what happens if we are getting interrupted by NMI? > > I think we can do without the spinlock. I think I was already told that when > I tried to put it [some where else] in v1. > > It's fun failing in public ;) another note is that printk()->vscnprintf()->get_random_bytes()->warn_unseeded_randomness() causes a printk() recursion, but we should be fine now, we are in printk_safe mode by the time we vscnprintf(). but a bigger problem might the following thing: vscnprintf() pointer() ptr_to_id() initialize_ptr_secret() get_random_bytes() _get_random_bytes() extract_crng() _extract_crng() spin_lock_irqsave(&crng->lock, flags); <<<<< this, once again, can deadlock. can it? just like before: > > printk() > > vprintk_emit() > > vscnprintf() > > pointer() > > ptr_to_id() > > initialize_ptr_secret() > > spin_lock(&key_lock) > > > > ----> NMI > > > > printk() > > printk_safe_log_store() > > vscnprintf() > > pointer() > > ptr_to_id() > > initialize_ptr_secret() > > spin_lock(&key_lock) <<<< -ss