[PATCH] sparse: __user annotations for ipc compat code

[PATCH] sparse: __user annotations for ipc compat code

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 2830 bytes --]

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 <arnd@arndb.de>

===== 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)

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[viro]

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...