Skip to contentSkip to conversation

sparse: __user annotations for ipc compat code

PATCH In review Arnd Bergmann 2 participants 2 messages Jun 11, 2004

lore
Share & tools
lore

Arnd Bergmann

author
#0 lore raw

PATCH 1/1 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 =====

ipc/compat.c +6 -6
@@ -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 =====

ipc/compat_mq.c +8 -4
@@ -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)
#1 lore raw

Review

Quoted 2 lines · Arnd Bergmann
(quoted diff) +1 -1
-		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!

ipc/compat.c line 339

Quoted 5 lines
ipc/compat.c +1 -1
 	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...