| Message ID | CAOMZO5DZui3x554WcHsseSfXDgYeaq1va-QzVy1CQajE3M3=kQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Aug 3, 2012 at 1:46 PM, Fabio Estevam <festevam@gmail.com> wrote: > Marek, > > On Fri, Aug 3, 2012 at 12:55 PM, Marek Vasut <marex@denx.de> wrote: >> Initially reported by Detlev Zundel <dzu@denx.de>, this caused breakage >> when "su - testuser" was called. "testuser" being a regular user account, >> the command ended with SIGKILL. Also, please point to the commit that introduced such breakage. Thanks, Fabio Estevam
On Fri, Aug 3, 2012 at 1:46 PM, Fabio Estevam <festevam@gmail.com> wrote: > Marek, > > On Fri, Aug 3, 2012 at 12:55 PM, Marek Vasut <marex@denx.de> wrote: >> Initially reported by Detlev Zundel <dzu@denx.de>, this caused breakage >> when "su - testuser" was called. "testuser" being a regular user account, >> the command ended with SIGKILL. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: Shawn Guo <shawn.guo@linaro.org> >> Cc: Wolfgang Denk <wd@denx.de> >> --- >> arch/arm/configs/mxs_defconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig >> index ccdb635..f212802 100644 >> --- a/arch/arm/configs/mxs_defconfig >> +++ b/arch/arm/configs/mxs_defconfig >> @@ -34,7 +34,7 @@ CONFIG_NO_HZ=y >> CONFIG_HIGH_RES_TIMERS=y >> CONFIG_PREEMPT_VOLUNTARY=y >> CONFIG_AEABI=y >> -CONFIG_DEFAULT_MMAP_MIN_ADDR=65536 >> +CONFIG_DEFAULT_MMAP_MIN_ADDR=32768 > > No need to set it to 32768. > > If you just remove the 'CONFIG_DEFAULT_MMAP_MIN_ADDR=65536' then this > symbol will be 32768. Sorry, ignore what I said. I just realized that the default is 4096.
On Fri, Aug 03, 2012 at 01:54:07PM -0300, Fabio Estevam wrote: > On Fri, Aug 3, 2012 at 1:46 PM, Fabio Estevam <festevam@gmail.com> wrote: > > Marek, > > > > On Fri, Aug 3, 2012 at 12:55 PM, Marek Vasut <marex@denx.de> wrote: > >> Initially reported by Detlev Zundel <dzu@denx.de>, this caused breakage > >> when "su - testuser" was called. "testuser" being a regular user account, > >> the command ended with SIGKILL. > >> > >> Signed-off-by: Marek Vasut <marex@denx.de> > >> Cc: Russell King <linux@arm.linux.org.uk> > >> Cc: Shawn Guo <shawn.guo@linaro.org> > >> Cc: Wolfgang Denk <wd@denx.de> > >> --- > >> arch/arm/configs/mxs_defconfig | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig > >> index ccdb635..f212802 100644 > >> --- a/arch/arm/configs/mxs_defconfig > >> +++ b/arch/arm/configs/mxs_defconfig > >> @@ -34,7 +34,7 @@ CONFIG_NO_HZ=y > >> CONFIG_HIGH_RES_TIMERS=y > >> CONFIG_PREEMPT_VOLUNTARY=y > >> CONFIG_AEABI=y > >> -CONFIG_DEFAULT_MMAP_MIN_ADDR=65536 > >> +CONFIG_DEFAULT_MMAP_MIN_ADDR=32768 > > > > No need to set it to 32768. > > > > If you just remove the 'CONFIG_DEFAULT_MMAP_MIN_ADDR=65536' then this > > symbol will be 32768. > > Sorry, ignore what I said. I just realized that the default is 4096. 4096 is also fine for ARM too. There's not much point in having defconfigs change it - that would just be pure noise in the config files.
On Fri, Aug 3, 2012 at 2:40 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > 4096 is also fine for ARM too. There's not much point in having > defconfigs change it - that would just be pure noise in the config > files. Thanks for the clarification, Russell. Marek, so maybe you can send a v2 of this patch with my previous suggestion (and also mentioning the offending commit). I have also removed CONFIG_DEFAULT_MMAP_MIN_ADDR from imx_v6_v7_defconfig. Regards, Fabio Estevam
Dear Russell King - ARM Linux, [...] > > > No need to set it to 32768. > > > > > > If you just remove the 'CONFIG_DEFAULT_MMAP_MIN_ADDR=65536' then this > > > symbol will be 32768. > > > > Sorry, ignore what I said. I just realized that the default is 4096. > > 4096 is also fine for ARM too. There's not much point in having > defconfigs change it - that would just be pure noise in the config > files. Wasn't there a security concern being the reason for setting this higher? Also, I still don't completely understand why ARM has to set it lower than others, is that an ABI thing? Russell, do you happen to have some further reading for me I could study on this topic please? Thanks! Best regards, Marek Vasut
On Fri, Aug 03, 2012 at 08:46:27PM +0200, Marek Vasut wrote: > Dear Russell King - ARM Linux, > > [...] > > > > > No need to set it to 32768. > > > > > > > > If you just remove the 'CONFIG_DEFAULT_MMAP_MIN_ADDR=65536' then this > > > > symbol will be 32768. > > > > > > Sorry, ignore what I said. I just realized that the default is 4096. > > > > 4096 is also fine for ARM too. There's not much point in having > > defconfigs change it - that would just be pure noise in the config > > files. > > Wasn't there a security concern being the reason for setting this higher? I don't believe there is. The only requirement is that the first page on older CPUs isn't stomped on (and we preserve that for later CPUs so that NULL pointer derefs get caught.) The higher it is the better though, because it means NULL pointer + offset deref with larger offsets also gets caught. > Also, > I still don't completely understand why ARM has to set it lower than > others, is that an ABI thing? Yes, we have always loaded user programs at 0x8000 by default, which are generally not relocatable. So if you set it to 64K, you prevent non-root user programs being mapped in, which is why stuff gets killed as soon as UID != 0.
Dear Russell King - ARM Linux, > On Fri, Aug 03, 2012 at 08:46:27PM +0200, Marek Vasut wrote: > > Dear Russell King - ARM Linux, > > > > [...] > > > > > > > No need to set it to 32768. > > > > > > > > > > If you just remove the 'CONFIG_DEFAULT_MMAP_MIN_ADDR=65536' then > > > > > this symbol will be 32768. > > > > > > > > Sorry, ignore what I said. I just realized that the default is 4096. > > > > > > 4096 is also fine for ARM too. There's not much point in having > > > defconfigs change it - that would just be pure noise in the config > > > files. > > > > Wasn't there a security concern being the reason for setting this higher? > > I don't believe there is. The only requirement is that the first page > on older CPUs isn't stomped on (and we preserve that for later CPUs so > that NULL pointer derefs get caught.) > > The higher it is the better though, because it means NULL pointer + offset > deref with larger offsets also gets caught. Understood! > > Also, > > I still don't completely understand why ARM has to set it lower than > > others, is that an ABI thing? > > Yes, we have always loaded user programs at 0x8000 by default, which are > generally not relocatable. So if you set it to 64K, you prevent non-root > user programs being mapped in, which is why stuff gets killed as soon as > UID != 0. I see! Thanks for explaining! Best regards, Marek Vasut
diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig index ccdb635..4edcfb4 100644 --- a/arch/arm/configs/mxs_defconfig +++ b/arch/arm/configs/mxs_defconfig @@ -34,7 +34,6 @@ CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y CONFIG_PREEMPT_VOLUNTARY=y CONFIG_AEABI=y -CONFIG_DEFAULT_MMAP_MIN_ADDR=65536 CONFIG_AUTO_ZRELADDR=y CONFIG_FPE_NWFPE=y CONFIG_NET=y