History

compat: cleanup coding in compat_get_bitmap() and compat_put_bitmap()

In the functions compat_get_bitmap() and compat_put_bitmap() the variable nr_compat_longs stores how many compat_ulong_t words should be copied in a loop. The copy loop itself is this: if (nr_compat_longs-- > 0) { if (__get_user(um, umask)) return -EFAULT; } else { um = 0; } Since nr_compat_longs gets unconditionally decremented in each loop and since it's type is unsigned this could theoretically lead to out of bounds accesses to userspace if nr_compat_longs wraps around to (unsigned)(-1). Although the callers currently do not trigger out-of-bounds accesses, we should better implement the loop in a safe way to completely avoid such warp-arounds.

Helge Deller committed 11 years ago

Commit 9b7b8191 parent ff25ea81 file · +4 −2Browse files

Unified · Split

kernel/compat.c+4−2
@@ -912,7 +912,8 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask, · 911 unmodified lines
912912 * bitmap. We must however ensure the end of the
913913 * kernel bitmap is zeroed.
914914 */
915− if (nr_compat_longs-- > 0) {
915+ if (nr_compat_longs) {
916+ nr_compat_longs--;
916917 if (__get_user(um, umask))
917918 return -EFAULT;
918919 } else {
@@ -954,7 +955,8 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask, · 35 unmodified lines
954955 * We dont want to write past the end of the userspace
955956 * bitmap.
956957 */
957− if (nr_compat_longs-- > 0) {
958+ if (nr_compat_longs) {
959+ nr_compat_longs--;
958960 if (__put_user(um, umask))
959961 return -EFAULT;
960962 }