All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Fenghua Yu <fenghua.yu@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	linux-ia64@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>,
	kexec@lists.infradead.org
Subject: Re: [PATCH 2/3] kexec: ensure user memory sizes do not wrap
Date: Mon, 18 Apr 2016 18:17:56 +0800	[thread overview]
Message-ID: <20160418101756.GD3602@x1.redhat.com> (raw)
In-Reply-To: <20160418083715.GZ19428@n2100.arm.linux.org.uk>

On 04/18/16 at 09:37am, Russell King - ARM Linux wrote:
> On Mon, Apr 18, 2016 at 01:35:34PM +0800, Baoquan He wrote:
> > On 04/14/16 at 09:00pm, Russell King wrote:
> > > Ensure that user memory sizes do not wrap around when validating the
> > > user input, which can lead to the following input validation working
> > > incorrectly.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > >  kernel/kexec_core.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > index 8d34308ea449..d719a4d0ef55 100644
> > > --- a/kernel/kexec_core.c
> > > +++ b/kernel/kexec_core.c
> > > @@ -169,6 +169,8 @@ int sanity_check_segment_list(struct kimage *image)
> > >  
> > >  		mstart = image->segment[i].mem;
> > >  		mend   = mstart + image->segment[i].memsz;
> > > +		if (mstart > mend)
> > > +			return result;
> > 
> > These segments are built in kexec utility, their availability should be
> > guaranteed there. If without some alignment handling, wrapping around
> > might not happen here. But I don't have strong objection to it.
> 
> In which case, what's the point of all the other validation which is done,
> like the check below:
> 
> > >  		if ((mstart & ~PAGE_MASK) || (mend & ~PAGE_MASK))
> > >  			return result;
> > >  		if (mend >= KEXEC_DESTINATION_MEMORY_LIMIT)
> 
> Your reply is contradictory to the whole suite of tests which kexec does
> to validate its input from userspace.

It's not contradictory. In kexec utility it will call
valid_memory_segment() to check each segment. And there it will check if
the start is bigger than end. What I meant is if start is 5000, end is
5100, an alignment of end will make start> end case happen. Anyway I am
fine with this check adding, the safer, the better.

> 
> -- 
> RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Fenghua Yu <fenghua.yu@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	linux-ia64@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>,
	kexec@lists.infradead.org
Subject: Re: [PATCH 2/3] kexec: ensure user memory sizes do not wrap
Date: Mon, 18 Apr 2016 10:17:56 +0000	[thread overview]
Message-ID: <20160418101756.GD3602@x1.redhat.com> (raw)
In-Reply-To: <20160418083715.GZ19428@n2100.arm.linux.org.uk>

On 04/18/16 at 09:37am, Russell King - ARM Linux wrote:
> On Mon, Apr 18, 2016 at 01:35:34PM +0800, Baoquan He wrote:
> > On 04/14/16 at 09:00pm, Russell King wrote:
> > > Ensure that user memory sizes do not wrap around when validating the
> > > user input, which can lead to the following input validation working
> > > incorrectly.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > >  kernel/kexec_core.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > index 8d34308ea449..d719a4d0ef55 100644
> > > --- a/kernel/kexec_core.c
> > > +++ b/kernel/kexec_core.c
> > > @@ -169,6 +169,8 @@ int sanity_check_segment_list(struct kimage *image)
> > >  
> > >  		mstart = image->segment[i].mem;
> > >  		mend   = mstart + image->segment[i].memsz;
> > > +		if (mstart > mend)
> > > +			return result;
> > 
> > These segments are built in kexec utility, their availability should be
> > guaranteed there. If without some alignment handling, wrapping around
> > might not happen here. But I don't have strong objection to it.
> 
> In which case, what's the point of all the other validation which is done,
> like the check below:
> 
> > >  		if ((mstart & ~PAGE_MASK) || (mend & ~PAGE_MASK))
> > >  			return result;
> > >  		if (mend >= KEXEC_DESTINATION_MEMORY_LIMIT)
> 
> Your reply is contradictory to the whole suite of tests which kexec does
> to validate its input from userspace.

It's not contradictory. In kexec utility it will call
valid_memory_segment() to check each segment. And there it will check if
the start is bigger than end. What I meant is if start is 5000, end is
5100, an alignment of end will make start> end case happen. Anyway I am
fine with this check adding, the safer, the better.

> 
> -- 
> RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

  reply	other threads:[~2016-04-18 10:18 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-14 19:59 [PATCH 0/3] Initial Kexec patches Russell King - ARM Linux
2016-04-14 19:59 ` Russell King - ARM Linux
2016-04-14 19:59 ` Russell King - ARM Linux
2016-04-14 20:00 ` [PATCH 1/3] kexec: don't invoke OOM-killer for control page allocation Russell King
2016-04-14 20:00   ` Russell King
2016-04-18  5:32   ` Baoquan He
2016-04-18  5:32     ` Baoquan He
2016-04-18  8:39     ` Russell King - ARM Linux
2016-04-18  8:39       ` Russell King - ARM Linux
2016-04-18 10:12       ` Baoquan He
2016-04-18 10:12         ` Baoquan He
2016-04-28  9:53         ` Baoquan He
2016-04-28  9:53           ` Baoquan He
2016-04-14 20:00 ` [PATCH 2/3] kexec: ensure user memory sizes do not wrap Russell King
2016-04-14 20:00   ` Russell King
2016-04-18  5:35   ` Baoquan He
2016-04-18  5:35     ` Baoquan He
2016-04-18  8:37     ` Russell King - ARM Linux
2016-04-18  8:37       ` Russell King - ARM Linux
2016-04-18 10:17       ` Baoquan He [this message]
2016-04-18 10:17         ` Baoquan He
2016-04-28  9:56   ` Baoquan He
2016-04-28  9:56     ` Baoquan He
2016-04-28 11:07   ` Minfei Huang
2016-04-28 11:07     ` Minfei Huang
2016-04-28 12:22     ` Russell King - ARM Linux
2016-04-28 12:22       ` Russell King - ARM Linux
2016-04-29  9:32       ` Minfei Huang
2016-04-29  9:32         ` Minfei Huang
2016-04-29  9:30         ` Russell King - ARM Linux
2016-04-29  9:30           ` Russell King - ARM Linux
2016-04-29 10:45           ` Minfei Huang
2016-04-29 10:45             ` Minfei Huang
2016-04-14 20:00 ` [PATCH 3/3] kexec: arrange for paddr_vmcoreinfo_note() to return phys_addr_t Russell King
2016-04-14 20:00   ` Russell King
2016-04-18  5:38   ` Baoquan He
2016-04-18  5:38     ` Baoquan He
2016-04-18  8:36     ` Russell King - ARM Linux
2016-04-18  8:36       ` Russell King - ARM Linux
2016-04-18 10:32       ` Baoquan He
2016-04-18 10:32         ` Baoquan He
2016-04-18 10:52         ` Russell King - ARM Linux
2016-04-18 10:52           ` Russell King - ARM Linux
2016-04-18 11:28           ` Baoquan He
2016-04-18 11:28             ` Baoquan He
2016-04-28  8:56             ` Russell King - ARM Linux
2016-04-28  8:56               ` Russell King - ARM Linux
2016-04-28  9:59   ` Baoquan He
2016-04-28  9:59     ` Baoquan He

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=20160418101756.GD3602@x1.redhat.com \
    --to=bhe@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=fenghua.yu@intel.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=tony.luck@intel.com \
    /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.