Hello, On Tue, Jun 03, 2014 at 09:51:33AM +0200, Uwe Kleine-König wrote: > On Mon, Jun 02, 2014 at 06:53:43PM +0200, Rabin Vincent wrote: > > On Mon, Apr 28, 2014 at 09:51:49AM +0200, Uwe Kleine-König wrote: > > > On Mon, Apr 21, 2014 at 08:10:08PM +0200, Rabin Vincent wrote: > > > > 8c56cc8be5b38e ("ARM: 7449/1: use generic strnlen_user and > > > > strncpy_from_user functions") apparently broken those string operations > > > > for !MMU. USER_DS == KERNEL_DS on !MMU, so user_addr_max() always > > > > restricts the addresses to TASK_SIZE. > > > > > > > > TASK_SIZE has anyway no meaning on !MMU, so make user_addr_max() not > > > > restrict anything. > > > > > > > > Signed-off-by: Rabin Vincent <rabin@xxxxxx> > > > I tested this on my efm32 machine and it booted just fine. Before I used > > > a patch that did: > > > > > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > > > index 02fa2558f662..f25c7f4c5a44 100644 > > > --- a/arch/arm/include/asm/memory.h > > > +++ b/arch/arm/include/asm/memory.h > > > @@ -92,9 +92,12 @@ > > > * It is difficult to define and perhaps will never meet the original meaning > > > * of this define that was meant to. > > > * Fortunately, there is no reference for this in noMMU mode, for now. > > > + * > > > + * HACK: copy_from_user must even handle copying from flash. So don't impose a > > > + * limit at all. Not sure this is correct ... > > > */ > > > #ifndef TASK_SIZE > > > -#define TASK_SIZE (CONFIG_DRAM_SIZE) > > > +#define TASK_SIZE (~0UL) > > > #endif > > > > The current code for user_addr_max() for !MMU is essentialy: > > > > #define user_addr_max() TASK_SIZE > > > > which is obviously wrong for the KERNEL_DS case, since it should be > > ~0UL. And user space can access all that the kernel does, so there > > should be no restriction for USER_DS either (which is anyway equivalent > > to KERNEL_DS). Hence, I think my patch, which removes the usage of > > TASK_SIZE in user_addr_max() for !MMU, is correct regardless of what the > > correct definition or meaning of TASK_SIZE for !MMU is. > > > > If you make TASK_SIZE to ~0UL (which is probably what it should be on > > !MMU), then the result is equivalent to my patch but it is not > > semantically correct since you are restricting user_addr_max() to > > TASK_SIZE even for the KERNEL_DS. > I'd prefer to share as much code as possible between MMU and !MMU, so my > preferred solution is: > > #ifndef CONFIG_MMU > #define TASK_SIZE ~0UL /* do we need parentesis? */ > #endif > > #define user_addr_max() \ > (segment_eq(get_fs(), KERNEL_DS) ? ~0UL : TASK_SIZE) After looking into that a bit more I wonder if the correct version is (maybe an equivalent to): #define user_addr_max() \ (segment_eq(get_fs(), KERNEL_DS) ? ~0UL : get_fs()) That is because in the MMU case get_fs() is #defined as: #define get_fs() (current_thread_info()->addr_limit) and .addr_limit is changeable via set_fs. This would also mean that the current definition: #define user_addr_max() \ (segment_eq(get_fs(), USER_DS) ? TASK_SIZE : ~0UL) might return ~0UL even though there is a limit which just happens not to be TASK_SIZE. (BTW, alpha, m68k, openrisc and sparc use the same definition.) Thoughts? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/linux-arm-kernel