From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc: Vincent Whitchurch <rabinv@axis.com>,
arnd@arndb.de, linux-kernel@vger.kernel.org,
akpm@linux-foundation.org, treding@nvidia.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] asm/sections: Check for overflow in memory_contains()
Date: Tue, 17 Dec 2019 10:28:31 +0000 [thread overview]
Message-ID: <20191217102831.GP25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <20191217102238.14792-1-vincent.whitchurch@axis.com>
On Tue, Dec 17, 2019 at 11:22:38AM +0100, Vincent Whitchurch wrote:
> ARM uses memory_contains() from its stacktrace code via this function:
>
> static inline bool in_entry_text(unsigned long addr)
> {
> return memory_contains(__entry_text_start, __entry_text_end,
> (void *)addr, 1);
> }
>
> addr is taken from the stack and can be a completely invalid. If addr
> is 0xffffffff, there is an overflow in the pointer arithmetic in
> memory_contains() and in_entry_text() incorrectly returns true.
>
> Fix this by adding an overflow check. The check is done on unsigned
> longs to avoid undefined behaviour.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
> include/asm-generic/sections.h | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index d1779d442aa5..e6e1b381c5df 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -105,7 +105,15 @@ static inline int arch_is_kernel_initmem_freed(unsigned long addr)
> static inline bool memory_contains(void *begin, void *end, void *virt,
> size_t size)
> {
> - return virt >= begin && virt + size <= end;
> + unsigned long membegin = (unsigned long)begin;
> + unsigned long memend = (unsigned long)end;
> + unsigned long objbegin = (unsigned long)virt;
> + unsigned long objend = objbegin + size;
> +
> + if (objend < objbegin)
> + return false;
> +
> + return objbegin >= membegin && objend <= memend;
Would merely changing to:
return virt >= begin && virt <= end - size;
be sufficient ? Is end - size possible to underflow?
> }
>
> /**
> --
> 2.20.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-12-17 10:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-17 10:22 [PATCH] asm/sections: Check for overflow in memory_contains() Vincent Whitchurch
2019-12-17 10:28 ` Russell King - ARM Linux admin [this message]
2019-12-18 14:49 ` Vincent Whitchurch
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=20191217102831.GP25745@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rabinv@axis.com \
--cc=treding@nvidia.com \
--cc=vincent.whitchurch@axis.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).