drivers/tty/vt: copy userspace arrays safely

The functions (v)memdup_user() are utilized to copy userspace arrays. This is done without overflow checks.

Use the new wrappers memdup_array_user() and vmemdup_array_user() to copy the arrays more safely.

drivers/tty/vt/keyboard.c +2 -2

 		if (ct) {
-			buf = memdup_user(a->kbdiacruc,
-					  ct * sizeof(struct kbdiacruc));
+			buf = memdup_array_user(a->kbdiacruc,
+						ct, sizeof(struct kbdiacruc));

Quoted 9 lines · Philipp Stanner

The functions (v)memdup_user() are utilized to copy userspace arrays. This is done without overflow checks.

Use the new wrappers memdup_array_user() and vmemdup_array_user() to copy the arrays more safely.

drivers/tty/vt/consolemap.c +0 -0

@@ -644,7 +644,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
      if (!ct)
              return 0;

drivers/tty/vt/consolemap.c int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) · line 649

Quoted 4 lines

drivers/tty/vt/consolemap.c +1 -1

-     unilist = vmemdup_user(list, array_size(sizeof(*unilist), ct));
+     unilist = vmemdup_array_user(list, ct, sizeof(*unilist));
      if (IS_ERR(unilist))
              return PTR_ERR(unilist);

a 16bit value times sizeof(something).

Quoted 17 lines · Al Viro

Quoted 14 lines · Philipp Stanner

The functions (v)memdup_user() are utilized to copy userspace arrays. This is done without overflow checks.

Use the new wrappers memdup_array_user() and vmemdup_array_user() to copy the arrays more safely.

drivers/tty/vt/consolemap.c +1 -1

@@ -644,7 +644,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
      if (!ct)
              return 0;
 
-     unilist = vmemdup_user(list, array_size(sizeof(*unilist), ct));
+     unilist = vmemdup_array_user(list, ct, sizeof(*unilist));
      if (IS_ERR(unilist))
              return PTR_ERR(unilist);

Quoted 1 line

a 16bit value times sizeof(something).

So since it's already using array_size here, moving it to a new helper for consistency just makes things clearer, and so you are fine with the patch?

Quoted 23 lines · David Airlie

Quoted 21 lines · Al Viro

Quoted 18 lines · Philipp Stanner

The functions (v)memdup_user() are utilized to copy userspace arrays.
This is done without overflow checks.

Use the new wrappers memdup_array_user() and
vmemdup_array_user() to
copy the arrays more safely.

drivers/tty/vt/consolemap.c +1 -1

@@ -644,7 +644,7 @@ int con_set_unimap(struct vc_data *vc,
 ushort ct, struct unipair __user *list)
       if (!ct)
               return 0;
 
-     unilist = vmemdup_user(list, array_size(sizeof(*unilist),
 ct));
+     unilist = vmemdup_array_user(list, ct, sizeof(*unilist));
       if (IS_ERR(unilist))

              return PTR_ERR(unilist);

Quoted 6 lines

Quoted 1 line

a 16bit value times sizeof(something).

So since it's already using array_size here, moving it to a new helper
for consistency just makes things clearer, and so you are fine with the patch?

Sigh...  OK, if you want it spelled out, there we go.  I have no objections
to the contents of patches; e.g. in case of ppp ioctl it saves the reader
a grep in search of structure definitions, which is a good thing. The one
and only suggestion I have for those patches is that such patches might be
better off with explicit "in this case the overflow is avoided due to <reasons>, but use of this helper makes it obviously safe" - or, in case
of real bugs, "the overflow is, indeed, possible here", in which case
Fixes: ... and Cc: stable might be in order.