lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171009235107.GD20083@eros>
Date:   Tue, 10 Oct 2017 10:51:07 +1100
From:   "Tobin C. Harding" <me@...in.cc>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     rkrcmar@...hat.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: remove printing of token address

On Mon, Oct 09, 2017 at 12:58:12PM +0200, Paolo Bonzini wrote:
> On 09/10/2017 12:04, Tobin C. Harding wrote:
> > On Mon, Oct 09, 2017 at 03:49:38AM -0400, Paolo Bonzini wrote:
> >>
> >>
> >> ----- Original Message -----
> >>> From: "Tobin C. Harding" <me@...in.cc>
> >>> To: "Paolo Bonzini" <pbonzini@...hat.com>, rkrcmar@...hat.com
> >>> Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, "Tobin C. Harding" <me@...in.cc>
> >>> Sent: Monday, October 9, 2017 8:30:14 AM
> >>> Subject: [PATCH] KVM: remove printing of token address
> >>>
> >>> KVM currently prints the address of the consumer token. It is not
> >>> immediately clear what benefit it is to see this address. Printing
> >>> this address leaks kernel pointers into dmesg and is a security risk.
> >>>
> >>> Remove the consumer token address from error message output.
> >>
> >> It should use %pK instead.
> > 
> > Is there any other way we can identify a token? There is some push back against kpt_restrict (as
> > used by %pK) at the moment. If there is another sane way to do it perhaps we could consider that,
> > else I'll use %pK for v2.
> 
> Not really, we know it is an eventfd but you can't go from the struct
> eventfd_ctx* (the token) to the corresponding struct file.
> 
> I'm not sure about the pushback... I've read your name in
> https://lwn.net/Articles/735589/ :) and that article says "the same
> effect as a restrictive kptr_restrict setting could be achieved by
> searching for (and fixing) every use of unadorned "%p" directives in the
> kernel".  As I understand it, the push back is against restrictive
> kptr_restrict settings, not against using "%pK" _to avoid the need_ for
> such a restrictive setting.

In the thread that article is based on Linus airs his view that kptr_restrict is fundamentally
broken. 

Would you be happy if instead of printing the pointer we printed a unique identifier (some suitable
hash of the pointer value)? I'm working on that patch ATM, if this sounds ok I'll leave this patch
until unique identifiers are implemented.

But if we really want the address here I can put in the patch using %pK. What is your thoughts?

> Thanks,
> 
> Paolo
> 
> >> Also, please do the same change on the VFIO
> >> side (drivers/vfio/pci/vfio_pci_intrs.c, call to irq_bypass_register_producer).

And same here obviously.

thanks,
Tobin.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ