All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Sasha Levin <levinsasha928@gmail.com>,
	arnd@arndb.de, viro@zeniv.linux.org.uk, davej@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kmsg: Use vmalloc instead of kmalloc when writing
Date: Fri, 30 Mar 2012 22:35:46 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LFD.2.02.1203302225420.2542@ionos> (raw)
In-Reply-To: <20120330164913.GA19946@kroah.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1475 bytes --]

On Fri, 30 Mar 2012, Greg KH wrote:
> On Fri, Mar 30, 2012 at 07:37:37PM +0300, Sasha Levin wrote:
> > On Fri, Mar 30, 2012 at 6:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Fri, Mar 30, 2012 at 01:04:27PM -0400, Sasha Levin wrote:
> > >> There are no size checks in kmsg_write(), and we try allocating enough
> > >> memory to store everything userspace gave us, which may be too much for
> > >> kmalloc to allocate.
> > >
> > > Really?  Have you seen this fail?  As only root can do this, is this
> > > really a problem?
> > 
> > Only root, and a whole bunch of management software that dumps data
> > into /dev/kmsg (systemd and friends).
> 
> Running as root, do any of these cause problems by asking for too much
> memory here? 

Running as root is not a guarantee for correctness. So the syscall
should cope with bogus requests from user space and not rely on the
sanity of anything. Looking at the main users which polute dmesg I'm
inclined to assume insanity in the first place.

As Sasha pointed out there is either the variant to use vmalloc and
grant any write size or limit the size to something sensible. Though
given the users of this, coming up with something sensible might be a
problem.

> Is this something that needs to be addressed now, and in
> stable kernels, or can it wait for 3.5?

Yes, it want's to be addressed now and it want's to be in stable as
well. syscalls which have no bound checking are evil, no matter what.
 
Thanks,

	tglx

  parent reply	other threads:[~2012-03-30 20:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-30 17:04 [PATCH] kmsg: Use vmalloc instead of kmalloc when writing Sasha Levin
2012-03-30 15:30 ` Greg KH
2012-03-30 16:37   ` Sasha Levin
2012-03-30 16:49     ` Greg KH
2012-03-30 17:15       ` Sasha Levin
2012-03-30 20:35       ` Thomas Gleixner [this message]
2012-03-30 20:42         ` Greg KH
2012-03-30 20:49           ` Thomas Gleixner
2012-03-30 21:05             ` Arnd Bergmann
2012-03-30 21:17               ` Greg KH
2012-03-30 22:02                 ` Sasha Levin
2012-03-30 23:43                   ` Greg KH
2012-03-31  0:02                     ` Kay Sievers
2012-03-31  8:57                       ` Sasha Levin
2012-04-23  9:54                     ` Sasha Levin
2012-03-30 21:18               ` Thomas Gleixner
2012-03-31  1:43               ` Joe Perches

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=alpine.LFD.2.02.1203302225420.2542@ionos \
    --to=tglx@linutronix.de \
    --cc=arnd@arndb.de \
    --cc=davej@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.