lkml.org 
[lkml]   [1999]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] 32-bit UID support for 2.3.30pre6 (important update)
Date

> Hmm. Would you imagine doing something like:
>
> asmlinkage long sys_msgctl (int msqid, int cmd, struct msqid_ds *buf)
> {
> ...
> int ipc_version = 0;
>
> if(cmd & NEW_IPC64_FLAG) {
> ipc_version = 1;
> cmd ^= NEW_IPC64_FLAG;
> }
>
Yes.

> My only concern with that is that it would obfuscate the locking in the
> code, specifically the dropping of the spinlock before doing a
> copy_to_user:
>
> if ( (cmd == MSG_STAT64) || (cmd == IPC_STAT64) ) {
> struct msqid64_ds tbuf;
> memset(&tbuf,0,sizeof(tbuf));
> kernel_to_msqid64_ds(msq, &tbuf);
> msg_unlock(msqid);
> if (copy_to_user (buf, &tbuf, sizeof(tbuf)))
> return -EFAULT;
>
> How about rewriting the code as follows:
>
> /* NOTE: copy_msqid_to_user does msg_unlock before the
> actual copy_to_user, dropping the spinlock so we can
> possibly sleep in copy_to_user */
>
> if (copy_msq_to_user(msq, buf, cmd))
> return -EFAULT;
No. A function should never release a lock.
I though about the following code:

struct msqid64_ds tbuf;
memset(&tbuf,0,sizeof(tbuf));
[...]
msg_unlock(msqid);
if (copy_msqid_to_user (buf, &tbuf, version))
return -EFAULT;

static void copy_msqid_to_user(void * buf, struct mgqid64_ds *msqid, int version)
{
msqid_ds old;

if(version == 1)
return copy_to_user(buf,msqid,sizeof(*msqid));
old.abc = msqid.abc;
[...]
return copy_to_user(buf,&old,sizeof(old));
}

The knowledge about the legacy support should be completely hidden
within copy_xxx_to_user().

--
Manfred

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:55    [W:0.055 / U:1.732 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site