ARM: uaccess: Implement strict user copy checks
in [Kernel]
|
Prev: [GIT PULL] Ceph updates for 2.6.36-rc1
Next: usb 2.0 em28xx kernel panic general protection fault: 0000 [#1] SMP RIP: 0010:[<ffffffffa004fbc5>] [<ffffffffa004fbc5>] em28xx_isoc_copy_vbi+0x62e/0x812 [em28xx]
From: Stephen Boyd on 10 Aug 2010 18:50 On 08/03/2010 08:02 PM, Stephen Boyd wrote: > This is mostly a copy from the s390 implementation (which copied > from x86 and sparc), except we print a warning if the Kconfig > option is disabled. > > Signed-off-by: Stephen Boyd <sboyd(a)codeaurora.org> > Ping? Should I submit this to the patch system? -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on 10 Aug 2010 19:00 On Tue, Aug 10, 2010 at 03:46:59PM -0700, Stephen Boyd wrote: > On 08/03/2010 08:02 PM, Stephen Boyd wrote: >> This is mostly a copy from the s390 implementation (which copied >> from x86 and sparc), except we print a warning if the Kconfig >> option is disabled. >> >> Signed-off-by: Stephen Boyd <sboyd(a)codeaurora.org> >> > > Ping? > > Should I submit this to the patch system? Unfortunately, there's quite a dearth of information on this patch, so I can't say. I think it needs some testing before a decision can be made. What compilers have been tested with this? As the help comments intimate that it needs at least gcc 4.4, and you've changed it to produce a compile time warning if the option is disabled, what's the implications for older compilers? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Boyd on 10 Aug 2010 20:30 On 08/10/2010 03:55 PM, Russell King - ARM Linux wrote: > > Unfortunately, there's quite a dearth of information on this patch, > so I can't say. I think it needs some testing before a decision can > be made. Ok. I'll add more info and resend. Do you mind testing ;-) > > What compilers have been tested with this? So far I've tested it with gcc-4.4.0 and 4.3.1 > > As the help comments intimate that it needs at least gcc 4.4, and > you've changed it to produce a compile time warning if the option is > disabled, what's the implications for older compilers? With older compilers (pre 4.4) __compiletime_object_size() will be replaced with -1 causing this code to be optimized away. Also, __compiletime_warning() and __compiletime_error() aren't defined to be anything except in include/linux/compiler-gcc4.h so users of older compilers shouldn't see any warnings or errors regardless of the config option being enabled. People will start seeing warnings if they use gcc 4.4 or later though. It's debatable whether or not to have both the warning and the error when you consider -Werror. I went this way since x86 and parisc opted for warnings and errors. Furthermore, I don't see any warnings or errors with this patch in my builds. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnd Bergmann on 10 Aug 2010 23:10 On Wednesday 04 August 2010, Stephen Boyd wrote: > +config DEBUG_STRICT_USER_COPY_CHECKS > + bool "Strict user copy size checks" > + depends on DEBUG_KERNEL > + help > + Enabling this option turns a certain set of sanity checks for user > + copy operations into compile time errors. > + > + The copy_from_user() etc checks are there to help test if there > + are sufficient security checks on the length argument of > + the copy operation, by having gcc prove that the argument is > + within bounds. > + > + If unsure, or if you run an older (pre 4.4) gcc, say N. > + Do you actually need to disable this if running an older gcc? AFAICT, it should just have no effect at all in that case, so the comment is slightly misleading. Also, why turn this specific warning into an error but not any of the other warnings? Some architectures (alpha, sparc, mips, powerpc, sh) simply turn on -Werror for architecture specific code in general, which seems very useful. We can also make that a config option (probably arch independent) that we turn on for defconfig files that we know build without warnings. Unfortunately, there is a number of device drivers that have never been warning-free, so we can't just enable -Werror for all code. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Boyd on 11 Aug 2010 14:50
On 08/10/2010 08:04 PM, Arnd Bergmann wrote: > > Do you actually need to disable this if running an older gcc? AFAICT, it > should just have no effect at all in that case, so the comment is slightly > misleading. I blindly copied the help text from x86. Will fix to be less misleading. > > Also, why turn this specific warning into an error but not any of the other > warnings? Some architectures (alpha, sparc, mips, powerpc, sh) simply turn > on -Werror for architecture specific code in general, which seems very > useful. We can also make that a config option (probably arch independent) > that we turn on for defconfig files that we know build without warnings. > > Unfortunately, there is a number of device drivers that have never been > warning-free, so we can't just enable -Werror for all code. > I'm following the x86 implementation. I suppose it's done this way since many drivers aren't warning free (as you mention) and turning on -Werror will make it more annoying to find these types of errors. Since there isn't any -Werror=user-copy this approach allows us to find this type of error easily without having to sift through noise. Enabling -Werror in architecture specific code wouldn't help much here though right since this is going to be inlined into drivers and such? -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |