2004-06-11 15:29:54

by Arnd Bergmann

[permalink] [raw]

Subject: [PATCH] sparse: __user annotations for ipc compat code

---

Add a few __user annotations to the ipc/compat* code.

compat.c | 12 ++++++------
compat_mq.c | 12 ++++++++----
2 files changed, 14 insertions(+), 10 deletions(-)

Signed-off-by: Arnd Bergmann <[email protected]>

===== ipc/compat.c 1.2 vs edited =====
--- 1.2/ipc/compat.c Sat May 29 11:12:55 2004
+++ edited/ipc/compat.c Sun Jun 6 23:03:35 2004
@@ -279,7 +279,7 @@

case IPC_STAT:
case SEM_STAT:
- fourth.__pad = &s64;
+ fourth.__pad = (void __user *)&s64;
err = do_semctl(first, second, third, fourth);
if (err < 0)
break;
@@ -302,7 +302,7 @@
if (err)
break;

- fourth.__pad = &s64;
+ fourth.__pad = (void __user *)&s64;
err = do_semctl(first, second, third, fourth);
break;

@@ -335,7 +335,7 @@
goto out;
old_fs = get_fs();
set_fs(KERNEL_DS);
- err = sys_msgsnd(first, p, second, third);
+ err = sys_msgsnd(first, (struct msgbuf __user *)p, second, third);
set_fs(old_fs);
out:
kfree(p);
@@ -374,7 +374,7 @@
goto out;
old_fs = get_fs();
set_fs(KERNEL_DS);
- err = sys_msgrcv(first, p, second, msgtyp, third);
+ err = sys_msgrcv(first, (struct msgbuf __user *)p, second, msgtyp, third);
set_fs(old_fs);
if (err < 0)
goto free_then_out;
@@ -457,7 +457,7 @@

old_fs = get_fs();
set_fs(KERNEL_DS);
- err = sys_msgctl(first, second, buf);
+ err = sys_msgctl(first, second, (struct msqid_ds __user *)buf);
set_fs(old_fs);

return err;
@@ -630,7 +630,7 @@

old_fs = get_fs();
set_fs(KERNEL_DS);
- err = sys_shmctl(shmid, cmd, buf);
+ err = sys_shmctl(shmid, cmd, (struct shmid_ds __user *)buf);
set_fs(old_fs);

return err;
===== ipc/compat_mq.c 1.2 vs edited =====
--- 1.2/ipc/compat_mq.c Sat May 29 11:12:55 2004
+++ edited/ipc/compat_mq.c Sun Jun 6 23:15:29 2004
@@ -67,7 +67,8 @@

oldfs = get_fs();
set_fs(KERNEL_DS);
- ret = sys_mq_open(name, oflag, mode, &attr);
+ ret = sys_mq_open((char __user *)name, oflag, mode,
+ (struct mq_attr __user *)&attr);
set_fs(oldfs);

putname(name);
@@ -86,7 +87,7 @@
u_ts = compat_alloc_user_space(sizeof(*u_ts));
if (get_compat_timespec(&ts, u_abs_timeout)
|| copy_to_user(u_ts, &ts, sizeof(*u_ts)))
- return ERR_PTR(-EFAULT);
+ return (void __user *)ERR_PTR(-EFAULT);

return u_ts;
}
@@ -161,7 +162,8 @@

oldfs = get_fs();
set_fs(KERNEL_DS);
- ret = sys_mq_notify(mqdes, &notification);
+ ret = sys_mq_notify(mqdes,
+ (const struct sigevent __user *)&notification);
set_fs(oldfs);

return ret;
@@ -187,7 +189,9 @@

oldfs = get_fs();
set_fs(KERNEL_DS);
- ret = sys_mq_getsetattr(mqdes, p_mqstat, p_omqstat);
+ ret = sys_mq_getsetattr(mqdes,
+ (const struct mq_attr __user *)p_mqstat,
+ (struct mq_attr __user *)p_omqstat);
set_fs(oldfs);

if (ret)

---

Attachments:

(No filename) (2.66 kB)
(No filename) (189.00 B)
signature Download all attachments

2004-06-11 18:31:22

by Al Viro

[permalink] [raw]

Subject: Re: [PATCH] sparse: __user annotations for ipc compat code

---

On Fri, Jun 11, 2004 at 05:27:30PM +0200, Arnd Bergmann wrote:
> - fourth.__pad = &s64;
> + fourth.__pad = (void __user *)&s64;

That makes absolutely no sense (and should generate a warning anyway).
This is _NOT_ a userland pointer. Obviously so - we are talking about
on-stack address, for crying out loud!

> old_fs = get_fs();
> set_fs(KERNEL_DS);
> - err = sys_msgsnd(first, p, second, third);
> + err = sys_msgsnd(first, (struct msgbuf __user *)p, second, third);
> set_fs(old_fs);

Again, makes no sense whatsoever (we _still_ get a warning and clear fix
would be to get rid of set_fs() here and switch to compat_alloc_user_space()).

Same goes for the rest of patch.

Folks, warnings are not personal performance metrics, they are tools for
finding bogus code. Sigh...