* AUDIT of 2.5.15 copy_to/from_user
@ 2002-05-19 4:18 Rusty Russell
2002-05-18 22:55 ` Arnaldo Carvalho de Melo
` (3 more replies)
0 siblings, 4 replies; 30+ messages in thread
From: Rusty Russell @ 2002-05-19 4:18 UTC (permalink / raw)
To: linux-kernel; +Cc: kernel-janitor-discuss
The following uses seem to be incorrect: copy_from_user and
copy_to_user return the number of bytes NOT copied on failure, not
-EFAULT.
You can CC: fixups to trivial at rustcorp.com.au.
(I didn't look for cases where the Torvalds/McVoy philosophy says we
should be returning a partial result on EFAULT: that's more complex).
Thanks,
Rusty.
================
Some cases are endemic: whole subsystems or drivers where the author
obviously thought copy_from_user follows the kernel conventions:
Whole Subsystems:
fs/intermezzo/*.c
sound/oss/*.c
sound/pci/*.c
Whole Drivers:
drivers/block/DAC960.c
drivers/block/cpqarray.c
drivers/block/swim3.c
drivers/block/swim_iop.c
drivers/char/rio/rioctrl.c
drivers/char/raw.c
drivers/isdn/icn/icn.c
drivers/isdn/capi/capi.c
drivers/isdn/capi/kcapi.c
drivers/isdn/sc/command.c
drivers/isdn/sc/ioctl.c
drivers/isdn/act2000/module.c
drivers/isdn/divert/divert_procfs.c
drivers/sbus/char/openprom.c
drivers/usb/class/audio.c
drivers/tc/zs.c
drivers/ieee1394/pcilynx.c
drivers/s390/misc/chandev.c
drivers/usb/input/hiddev.c
drivers/usb/media/dabusb.c
Lines:
drivers/char/nwflash.c:158: ret = copy_to_user(buf, (void *)(FLASH_BASE + p), count);
drivers/scsi/scsi_ioctl.c:383: return copy_to_user(arg, dev->host->pci_dev->slot_name,
drivers/sgi/char/sgiserial.c:1239: return copy_to_user(retinfo,&tmp,sizeof(*retinfo));
drivers/usb/misc/auerswald.c:1556: ret = copy_to_user(devinfo.buf, cp->dev_desc, u);
arch/i386/kernel/signal.c:37: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/sparc/kernel/signal.c:101: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/alpha/kernel/signal.c:44: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/sparc/kernel/sys_sunos.c:481: ret = copy_to_user(&name->sname[0], &system_utsname.sysname[0], sizeof(name->sname) - 1);
arch/mips/kernel/signal.c:45: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/ppc/kernel/signal.c:70: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/m68k/kernel/signal.c:198: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/sparc64/kernel/sys_sparc32.c:3675: return copy_to_user(res32, kres, sizeof(*res32));
arch/sparc64/kernel/signal.c:49: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/arm/kernel/signal.c:62: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/sh/kernel/signal.c:42: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/ia64/kernel/signal.c:147: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/mips64/kernel/linux32.c:1537: err |= __copy_from_user (p->mtext, &up->mtext, second);
arch/mips64/kernel/signal.c:45: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/s390/kernel/debug.c:458: if ((rc = copy_to_user(user_buf + count,
arch/s390/kernel/ptrace.c:119: retval=copy_from_user((void *)realuseraddr,(void *)copyaddr,len);
arch/s390/kernel/ptrace.c:345: if((ret=copy_from_user(&parea,(void *)addr,sizeof(parea)))==0)
arch/s390/kernel/signal.c:57: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/parisc/kernel/signal.c:44: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/cris/kernel/signal.c:51: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/s390x/kernel/debug.c:458: if ((rc = copy_to_user(user_buf + count,
arch/s390x/kernel/ptrace.c:119: retval = copy_from_user(realuserptr, copyptr, len);
arch/s390x/kernel/ptrace.c:360: if((ret=copy_from_user(&parea,(void *)addr,sizeof(parea)))==0)
arch/cris/kernel/signal.c:51: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/x86_64/ia32/ia32_signal.c:48: return __copy_to_user(to, from, sizeof(siginfo_t));
arch/x86_64/ia32/sys_ia32.c:2362: ret = copy_to_user(name->machine, "i386\0\0", 8);
arch/x86_64/ia32/sys_ia32.c:2971: return copy_to_user(res32, kres, sizeof(*res32));
arch/x86_64/kernel/signal.c:47: return __copy_to_user(to, from, sizeof(siginfo_t));
sound/isa/sb/sb16_csp.c:218: err = copy_to_user((void *) arg, &info, sizeof(info));
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 4:18 AUDIT of 2.5.15 copy_to/from_user Rusty Russell @ 2002-05-18 22:55 ` Arnaldo Carvalho de Melo 2002-05-18 23:12 ` [BKPATCH] " Arnaldo Carvalho de Melo 2002-05-18 23:54 ` Arnaldo Carvalho de Melo 2002-05-19 11:44 ` Alan Cox ` (2 subsequent siblings) 3 siblings, 2 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-18 22:55 UTC (permalink / raw) To: Rusty Russell; +Cc: linux-kernel, kernel-janitor-discuss Heads up: I'm finishing a bk changeset for intermezzo, will be submitting to Linus in some minutes. - Arnaldo Em Sun, May 19, 2002 at 02:18:53PM +1000, Rusty Russell escreveu: > The following uses seem to be incorrect: copy_from_user and > copy_to_user return the number of bytes NOT copied on failure, not > -EFAULT. > > You can CC: fixups to trivial at rustcorp.com.au. > > (I didn't look for cases where the Torvalds/McVoy philosophy says we > should be returning a partial result on EFAULT: that's more complex). > > Thanks, > Rusty. > ================ > Some cases are endemic: whole subsystems or drivers where the author > obviously thought copy_from_user follows the kernel conventions: > > Whole Subsystems: > fs/intermezzo/*.c <SNIP> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [BKPATCH] Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-18 22:55 ` Arnaldo Carvalho de Melo @ 2002-05-18 23:12 ` Arnaldo Carvalho de Melo 2002-05-18 23:54 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-18 23:12 UTC (permalink / raw) To: Linus Torvalds, Peter J.Braam Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 07:55:35PM -0300, Arnaldo C. Melo escreveu: > Heads up: I'm finishing a bk changeset for intermezzo, will be submitting > to Linus in some minutes. Here is, Linus, if you think its ok, please pull it from: http://kernel-acme.bkbits.net:8080/intermezzo-copy_tofrom_user-2.5 - Arnaldo # This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.540 -> 1.541 # fs/intermezzo/kml.c 1.1 -> 1.2 # fs/intermezzo/ext_attr.c 1.2 -> 1.3 # fs/intermezzo/psdev.c 1.7 -> 1.8 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/05/19 acme@conectiva.com.br 1.541 # fs/intermezzo/ext_attr.c # fs/intermezzo/kml.c # fs/intermezzo/psdev.c # # - fix copy_{to,from}_user error handling (thans to Rusty for pointing this out) # -------------------------------------------- # diff -Nru a/fs/intermezzo/ext_attr.c b/fs/intermezzo/ext_attr.c --- a/fs/intermezzo/ext_attr.c Sun May 19 02:07:36 2002 +++ b/fs/intermezzo/ext_attr.c Sun May 19 02:07:36 2002 @@ -105,9 +105,8 @@ printk("InterMezzo: out of memory!!!\n"); return -ENOMEM; } - error = copy_from_user(buf, buffer, buffer_len); - if (error) - return error; + if (copy_from_user(buf, buffer, buffer_len)) + return -EFAULT; } else buf = buffer; } else diff -Nru a/fs/intermezzo/kml.c b/fs/intermezzo/kml.c --- a/fs/intermezzo/kml.c Sun May 19 02:07:36 2002 +++ b/fs/intermezzo/kml.c Sun May 19 02:07:36 2002 @@ -31,10 +31,9 @@ ENTRY; /* allocate buffer & copy it to kernel space */ - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } if (input.reclen > kml_fsdata->kml_maxsize) @@ -45,11 +44,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.volname, input.namelen); - if ( error ) { + if (copy_from_user(path, input.volname, input.namelen)) { PRESTO_FREE(path, input.namelen + 1); EXIT; - return error; + return -EFAULT; } path[input.namelen] = '\0'; fset = kml_getfset (path); @@ -57,10 +55,9 @@ kml_fsdata = FSET_GET_KMLDATA(fset); /* read the buf from user memory here */ - error = copy_from_user(kml_fsdata->kml_buf, input.recbuf, input.reclen); - if ( error ) { + if (copy_from_user(kml_fsdata->kml_buf, input.recbuf, input.reclen)) { EXIT; - return error; + return -EFAULT; } kml_fsdata->kml_len = input.reclen; @@ -94,21 +91,19 @@ struct presto_file_set *fset; ENTRY; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.namelen + 1); if ( !path ) { EXIT; return -ENOMEM; } - error = copy_from_user(path, input.volname, input.namelen); - if ( error ) { + if (copy_from_user(path, input.volname, input.namelen)) { PRESTO_FREE(path, input.namelen + 1); EXIT; - return error; + return -EFAULT; } path[input.namelen] = '\0'; fset = kml_getfset (path); @@ -138,7 +133,8 @@ strlen (close->path) + 1, input.pathlen); error = -ENOMEM; } - copy_to_user((char *)arg, &input, sizeof (input)); + if (copy_to_user((char *)arg, &input, sizeof (input))) + return -EFAULT; } return error; } @@ -161,10 +157,9 @@ char *path; ENTRY; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.namelen + 1); @@ -172,11 +167,11 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.volname, input.namelen); + if (copy_from_user(path, input.volname, input.namelen)) { if ( error ) { PRESTO_FREE(path, input.namelen + 1); EXIT; - return error; + return -EFAULT; } path[input.namelen] = '\0'; fset = kml_getfset (path); @@ -193,7 +188,8 @@ #if 0 input.newpos = kml_upc->newpos; input.count = kml_upc->count; - copy_to_user((char *)arg, &input, sizeof (input)); + if (copy_to_user((char *)arg, &input, sizeof (input))) + return -EFAULT; #endif return error; } diff -Nru a/fs/intermezzo/psdev.c b/fs/intermezzo/psdev.c --- a/fs/intermezzo/psdev.c Sun May 19 02:07:36 2002 +++ b/fs/intermezzo/psdev.c Sun May 19 02:07:36 2002 @@ -149,9 +149,8 @@ return -EINVAL; } - error = copy_from_user(&hdr, buf, sizeof(hdr)); - if ( error ) - return error; + if (copy_from_user(&hdr, buf, sizeof(hdr))) + return -EFAULT; CDEBUG(D_PSDEV, "(process,opc,uniq)=(%d,%d,%d)\n", current->pid, hdr.opcode, hdr.unique); @@ -183,9 +182,8 @@ req->rq_bufsize, count, hdr.opcode, hdr.unique); count = req->rq_bufsize; /* don't have more space! */ } - error = copy_from_user(req->rq_data, buf, count); - if ( error ) - return error; + if (copy_from_user(req->rq_data, buf, count)) + return -EFAULT; /* adjust outsize: good upcalls can be aware of this */ req->rq_rep_size = count; @@ -280,14 +278,12 @@ char * tmp; int error; - error = copy_from_user(&readmount, (void *)arg, - sizeof(readmount)); - if ( error ) { + if (copy_from_user(&readmount, (void *)arg, sizeof(readmount))) printk("psdev: can't copy %Zd bytes from %p to %p\n", sizeof(readmount), (struct readmount *) arg, &readmount); EXIT; - return error; + return -EFAULT; } len = readmount.io_len; @@ -307,15 +303,16 @@ * I mean, let's let the compiler do a little work ... * gcc suggested the extra () */ - error = copy_to_user(readmount.io_string, tmp, outlen); - if ( error ) { + if (copy_to_user(readmount.io_string, tmp, outlen)) { CDEBUG(D_PSDEV, "Copy_to_user string 0x%p failed\n", readmount.io_string); + error = -EFAULT; } - if ((!error) && (error = copy_to_user(&(user_readmount->io_len), - &outlen, sizeof(int))) ) { + if (!error && copy_to_user(&(user_readmount->io_len), + &outlen, sizeof(int))) { CDEBUG(D_PSDEV, "Copy_to_user len @0x%p failed\n", &(user_readmount->io_len)); + error = -EFAULT; } PRESTO_FREE(tmp, len); @@ -360,10 +357,9 @@ int path_len; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.path_len + 1); @@ -371,11 +367,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.path, input.path_len); - if ( error ) { + if (copy_from_user(path, input.path, input.path_len)) { PRESTO_FREE(path, input.path_len + 1); EXIT; - return error; + return -EFAULT; } path[input.path_len] = '\0'; CDEBUG(D_PSDEV, "clear_fsetroot: path %s\n", path); @@ -401,10 +396,9 @@ int path_len; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.path_len + 1); @@ -412,11 +406,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.path, input.path_len); - if ( error ) { + if (copy_from_user(path, input.path, input.path_len)) { PRESTO_FREE(path, input.path_len + 1); EXIT; - return error; + return -EFAULT; } path[input.path_len] = '\0'; CDEBUG(D_PSDEV, "clear_all_fsetroot: path %s\n", path); @@ -440,10 +433,9 @@ int path_len; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.path_len + 1); @@ -451,11 +443,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.path, input.path_len); - if ( error ) { + if (copy_from_user(path, input.path, input.path_len)) { PRESTO_FREE(path, input.path_len + 1); EXIT; - return error; + return -EFAULT; } path[input.path_len] = '\0'; CDEBUG(D_PSDEV, "get_kmlsize: len %d path %s\n", @@ -474,7 +465,9 @@ CDEBUG(D_PSDEV, "get_kmlsize: size = %Zd\n", size); EXIT; - return copy_to_user((char *)arg, &input, sizeof(input)); + if (copy_to_user((char *)arg, &input, sizeof(input))) + return -EFAULT; + return 0; } case PRESTO_GET_RECNO: { @@ -488,10 +481,9 @@ int path_len; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.path_len + 1); @@ -499,11 +491,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.path, input.path_len); - if ( error ) { + if (copy_from_user(path, input.path, input.path_len)) { PRESTO_FREE(path, input.path_len + 1); EXIT; - return error; + return -EFAULT; } path[input.path_len] = '\0'; CDEBUG(D_PSDEV, "get_recno: len %d path %s\n", @@ -522,7 +513,9 @@ CDEBUG(D_PSDEV, "get_recno: recno = %d\n", (int) recno); EXIT; - return copy_to_user((char *)arg, &input, sizeof(input)); + if (copy_to_user((char *)arg, &input, sizeof(input))) + return -EFAULT; + return 0; } case PRESTO_SET_FSETROOT: { @@ -543,10 +536,9 @@ int flags; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } PRESTO_ALLOC(path, char *, input.path_len + 1); @@ -554,9 +546,9 @@ EXIT; return -ENOMEM; } - error = copy_from_user(path, input.path, input.path_len); - if ( error ) { + if (copy_from_user(path, input.path, input.path_len)) { EXIT; + error -EFAULT; goto exit_free_path; } path[input.path_len] = '\0'; @@ -567,9 +559,9 @@ EXIT; goto exit_free_path; } - error = copy_from_user(fsetname, input.name, input.name_len); - if ( error ) { + if (copy_from_user(fsetname, input.name, input.name_len)) { EXIT; + error = -EFAULT; goto exit_free_fsetname; } fsetname[input.name_len] = '\0'; @@ -621,12 +613,11 @@ struct psdev_opt *user_opt = (struct psdev_opt *) arg; int error; - error = copy_from_user(&kopt, (void *)arg, sizeof(kopt)); - if ( error ) { + if (copy_from_user(&kopt, (void *)arg, sizeof(kopt))) { printk("psdev: can't copyin %Zd bytes from %p to %p\n", sizeof(kopt), (struct kopt *) arg, &kopt); EXIT; - return error; + return -EFAULT; } minor = minor(dev); if (cmd == PRESTO_SETOPT) @@ -650,12 +641,11 @@ return error; } - error = copy_to_user(user_opt, &kopt, sizeof(kopt)); - if ( error ) { + if (copy_to_user(user_opt, &kopt, sizeof(kopt))) { CDEBUG(D_PSDEV, "Copy_to_user opt 0x%p failed\n", user_opt); EXIT; - return error; + return -EFAULT; } CDEBUG(D_PSDEV, "dosetopt minor %d, opt %d, val %d return %d\n", minor, kopt.optname, kopt.optval, error); @@ -668,10 +658,9 @@ struct lento_input_attr input; struct iattr iattr; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } iattr.ia_valid = input.valid; iattr.ia_mode = (umode_t)input.mode; @@ -692,10 +681,9 @@ int error; struct lento_input_mode input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_create(input.name, input.mode, &input.info); @@ -707,10 +695,9 @@ int error; struct lento_input_old_new input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_link(input.oldname, input.newname, &input.info); @@ -722,10 +709,9 @@ int error; struct lento_input input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_unlink(input.name, &input.info); @@ -737,10 +723,9 @@ int error; struct lento_input_old_new input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_symlink(input.oldname, input.newname,&input.info); @@ -752,10 +737,9 @@ int error; struct lento_input_mode input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_mkdir(input.name, input.mode, &input.info); @@ -767,10 +751,9 @@ int error; struct lento_input input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_rmdir(input.name, &input.info); @@ -782,10 +765,9 @@ int error; struct lento_input_dev input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_mknod(input.name, input.mode, @@ -798,10 +780,9 @@ int error; struct lento_input_old_new input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } error = lento_rename(input.oldname, input.newname, &input.info); @@ -817,30 +798,27 @@ char *name; char *buffer; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } /* Now setup the input parameters */ PRESTO_ALLOC(name, char *, input.name_len+1); /* We need null terminated strings for attr names */ name[input.name_len] = '\0'; - error=copy_from_user(name, input.name, input.name_len); - if ( error ) { + if (copy_from_user(name, input.name, input.name_len)) { EXIT; PRESTO_FREE(name,input.name_len+1); - return error; + return -EFAULT; } PRESTO_ALLOC(buffer, char *, input.buffer_len+1); - error=copy_from_user(buffer, input.buffer, input.buffer_len); - if ( error ) { + if (copy_from_user(buffer, input.buffer, input.buffer_len)) { EXIT; PRESTO_FREE(name,input.name_len+1); PRESTO_FREE(buffer,input.buffer_len+1); - return error; + return -EFAULT; } /* Make null terminated for easy printing */ buffer[input.buffer_len]='\0'; @@ -869,21 +847,19 @@ struct lento_input_ext_attr input; char *name; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } /* Now setup the input parameters */ PRESTO_ALLOC(name, char *, input.name_len+1); /* We need null terminated strings for attr names */ name[input.name_len] = '\0'; - error=copy_from_user(name, input.name, input.name_len); - if ( error ) { + if (copy_from_user(name, input.name, input.name_len)) { EXIT; PRESTO_FREE(name,input.name_len+1); - return error; + return -EFAULT; } CDEBUG(D_PSDEV," delextattr params: name %s," @@ -907,10 +883,9 @@ struct lento_input_iopen input; int error; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } input.fd = lento_iopen(input.name, (ino_t)input.ino, @@ -921,17 +896,18 @@ return input.fd; } EXIT; - return copy_to_user((char *)arg, &input, sizeof(input)); + if (copy_to_user((char *)arg, &input, sizeof(input))) + return -EFAULT; + return 0; } case PRESTO_VFS_CLOSE: { int error; struct lento_input_close input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } CDEBUG(D_PIOCTL, "lento_close file descriptor: %d\n", input.fd); @@ -952,10 +928,9 @@ struct presto_version remote_file_version; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } user_path = input.path; @@ -964,11 +939,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(input.path, user_path, input.path_len); - if ( error ) { + if (copy_from_user(input.path, user_path, input.path_len)) { EXIT; PRESTO_FREE(input.path, input.path_len + 1); - return error; + return -EFAULT; } input.path[input.path_len] = '\0'; @@ -996,10 +970,9 @@ struct lento_vfs_context info; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } user_path = input.path; @@ -1008,11 +981,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(input.path, user_path, input.path_len); - if ( error ) { + if (copy_from_user(input.path, user_path, input.path_len)) { EXIT; PRESTO_FREE(input.path, input.path_len + 1); - return error; + return -EFAULT; } input.path[input.path_len] = '\0'; @@ -1035,10 +1007,9 @@ __u32 path_len; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } user_path = input.path; @@ -1047,11 +1018,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(input.path, user_path, input.path_len); - if ( error ) { + if (copy_from_user(input.path, user_path, input.path_len)) { EXIT; PRESTO_FREE(input.path, input.path_len + 1); - return error; + return -EFAULT; } input.path[input.path_len] = '\0'; @@ -1072,10 +1042,9 @@ __u32 recno; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } user_path = input.path; @@ -1084,11 +1053,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(input.path, user_path, input.path_len); - if ( error ) { + if (copy_from_user(input.path, user_path, input.path_len)) { EXIT; PRESTO_FREE(input.path, input.path_len + 1); - return error; + return -EFAULT; } input.path[input.path_len] = '\0'; @@ -1111,10 +1079,9 @@ char *path; } input; - error = copy_from_user(&input, (char *)arg, sizeof(input)); - if ( error ) { + if (copy_from_user(&input, (char *)arg, sizeof(input))) { EXIT; - return error; + return -EFAULT; } user_path = input.path; @@ -1123,11 +1090,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(input.path, user_path, input.path_len); - if ( error ) { + if (copy_from_user(input.path, user_path, input.path_len)) { EXIT; PRESTO_FREE(input.path, input.path_len + 1); - return error; + return -EFAULT; } input.path[input.path_len] = '\0'; @@ -1190,7 +1156,9 @@ } /* return the correct cookie to wait for */ input.mark_what = res; - return copy_to_user((char *)arg, &input, sizeof(input)); + if (copy_to_user((char *)arg, &input, sizeof(input))) + return -EFAULT; + return 0; } #ifdef CONFIG_KREINT @@ -1211,11 +1179,10 @@ char *path; } permit; - error = copy_from_user(&permit, (char *)arg, sizeof(permit)); - if ( error ) { + if (copy_from_user(&permit, (char *)arg, sizeof(permit))) { EXIT; - return error; - } + return -EFAULT; + } user_path = permit.path; PRESTO_ALLOC(permit.path, char *, permit.path_len + 1); @@ -1223,11 +1190,10 @@ EXIT; return -ENOMEM; } - error = copy_from_user(permit.path, user_path, permit.path_len); - if ( error ) { + if (copy_from_user(permit.path, user_path, permit.path_len)) { EXIT; PRESTO_FREE(permit.path, permit.path_len + 1); - return error; + return -EFAULT; } permit.path[permit.path_len] = '\0'; @@ -1241,7 +1207,9 @@ return error; } /* return the correct cookie to wait for */ - return copy_to_user((char *)arg, &permit, sizeof(permit)); + if (copy_to_user((char *)arg, &permit, sizeof(permit))) + return -EFAULT; + return 0; } default: ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-18 22:55 ` Arnaldo Carvalho de Melo 2002-05-18 23:12 ` [BKPATCH] " Arnaldo Carvalho de Melo @ 2002-05-18 23:54 ` Arnaldo Carvalho de Melo 2002-05-19 0:14 ` [BKPATCH] OSS: " Arnaldo Carvalho de Melo 2002-05-19 0:19 ` Arnaldo Carvalho de Melo 1 sibling, 2 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-18 23:54 UTC (permalink / raw) To: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 07:55:35PM -0300, Arnaldo C. Melo escreveu: > Heads up: I'm finishing a bk changeset for intermezzo, will be submitting > to Linus in some minutes. Second heads up: OSS will be on its way to Linus in some minutes... - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
* [BKPATCH] OSS: Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-18 23:54 ` Arnaldo Carvalho de Melo @ 2002-05-19 0:14 ` Arnaldo Carvalho de Melo 2002-05-19 0:19 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 0:14 UTC (permalink / raw) To: Linus Torvalds; +Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 08:54:19PM -0300, Arnaldo C. Melo escreveu: > Em Sat, May 18, 2002 at 07:55:35PM -0300, Arnaldo C. Melo escreveu: > > Heads up: I'm finishing a bk changeset for intermezzo, will be submitting > > to Linus in some minutes. > > Second heads up: > > OSS will be on its way to Linus in some minutes... Here it is the OSS one... Linus, please consider pulling it from: http://kernel-acme.bkbits.net:8080/oss-copy_tofrom_user-2.5 - Arnaldo # This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.540 -> 1.540.1.1 # sound/oss/cmpci.c 1.12 -> 1.13 # sound/oss/cs46xx.c 1.17 -> 1.18 # sound/oss/msnd_pinnacle.c 1.7 -> 1.8 # sound/oss/esssolo1.c 1.12 -> 1.13 # sound/oss/ite8172.c 1.6 -> 1.7 # sound/oss/es1371.c 1.11 -> 1.12 # sound/oss/gus_wave.c 1.4 -> 1.5 # sound/oss/sb_audio.c 1.3 -> 1.4 # sound/oss/via82cxxx_audio.c 1.20 -> 1.21 # sound/oss/maestro.c 1.10 -> 1.11 # sound/oss/rme96xx.c 1.6 -> 1.7 # sound/oss/midibuf.c 1.3 -> 1.4 # sound/oss/sonicvibes.c 1.11 -> 1.12 # sound/oss/emu10k1/passthrough.c 1.4 -> 1.5 # sound/oss/es1370.c 1.12 -> 1.13 # sound/oss/sequencer.c 1.2 -> 1.3 # sound/oss/vwsnd.c 1.4 -> 1.5 # sound/oss/wavfront.c 1.10 -> 1.11 # sound/oss/emu10k1/cardwi.c 1.5 -> 1.6 # sound/oss/i810_audio.c 1.23 -> 1.24 # sound/oss/trident.c 1.20 -> 1.21 # sound/oss/emu10k1/cardwo.c 1.6 -> 1.7 # sound/oss/maestro3.c 1.16 -> 1.17 # sound/oss/cs4281/cs4281m.c 1.13 -> 1.14 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/05/19 acme@conectiva.com.br 1.541 # fs/intermezzo/ext_attr.c # fs/intermezzo/kml.c # fs/intermezzo/psdev.c # # - fix copy_{to,from}_user error handling (thans to Rusty for pointing this out) # -------------------------------------------- # 02/05/19 acme@conectiva.com.br 1.540.1.1 # drivers/sound/*.c # # - fix copy_{to,from}_user error handling (thanks to Rusty for pointing this out) # -------------------------------------------- # diff -Nru a/sound/oss/cmpci.c b/sound/oss/cmpci.c --- a/sound/oss/cmpci.c Sun May 19 03:08:22 2002 +++ b/sound/oss/cmpci.c Sun May 19 03:08:22 2002 @@ -2032,7 +2032,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -2049,7 +2051,9 @@ s->dma_adc.count &= s->dma_adc.fragsize-1; } spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/sound/oss/cs4281/cs4281m.c b/sound/oss/cs4281/cs4281m.c --- a/sound/oss/cs4281/cs4281m.c Sun May 19 03:08:22 2002 +++ b/sound/oss/cs4281/cs4281m.c Sun May 19 03:08:22 2002 @@ -3496,7 +3496,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize - 1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *) arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *) arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -3520,7 +3522,9 @@ if (s->dma_dac.mapped) s->dma_dac.count &= s->dma_dac.fragsize - 1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *) arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *) arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/sound/oss/cs46xx.c b/sound/oss/cs46xx.c --- a/sound/oss/cs46xx.c Sun May 19 03:08:22 2002 +++ b/sound/oss/cs46xx.c Sun May 19 03:08:22 2002 @@ -2965,7 +2965,9 @@ cinfo.blocks = dmabuf->count/dmabuf->divisor >> dmabuf->fragshift; cinfo.ptr = dmabuf->hwptr/dmabuf->divisor; spin_unlock_irqrestore(&state->card->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; } return -ENODEV; @@ -2998,7 +3000,9 @@ "cs46xx: GETOPTR bytes=%d blocks=%d ptr=%d\n", cinfo.bytes,cinfo.blocks,cinfo.ptr) ); spin_unlock_irqrestore(&state->card->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; } return -ENODEV; diff -Nru a/sound/oss/emu10k1/cardwi.c b/sound/oss/emu10k1/cardwi.c --- a/sound/oss/emu10k1/cardwi.c Sun May 19 03:08:22 2002 +++ b/sound/oss/emu10k1/cardwi.c Sun May 19 03:08:22 2002 @@ -304,9 +304,10 @@ static void copy_block(u8 *dst, u8 * src, u32 str, u32 len, u8 cov) { - if (cov == 1) - __copy_to_user(dst, src + str, len); - else { + if (cov == 1) { + if (__copy_to_user(dst, src + str, len)) + return; + } else { u8 byte; u32 i; @@ -314,7 +315,8 @@ for (i = 0; i < len; i++) { byte = src[2 * i] ^ 0x80; - __copy_to_user(dst + i, &byte, 1); + if (__copy_to_user(dst + i, &byte, 1)) + return; } } } diff -Nru a/sound/oss/emu10k1/cardwo.c b/sound/oss/emu10k1/cardwo.c --- a/sound/oss/emu10k1/cardwo.c Sun May 19 03:08:22 2002 +++ b/sound/oss/emu10k1/cardwo.c Sun May 19 03:08:22 2002 @@ -483,14 +483,17 @@ if (len > PAGE_SIZE - pgoff) { k = PAGE_SIZE - pgoff; - __copy_from_user((u8 *)dst[pg] + pgoff, src, k); + if (__copy_from_user((u8 *)dst[pg] + pgoff, src, k)) + return; len -= k; while (len > PAGE_SIZE) { - __copy_from_user(dst[++pg], src + k, PAGE_SIZE); + if (__copy_from_user(dst[++pg], src + k, PAGE_SIZE)) + return; k += PAGE_SIZE; len -= PAGE_SIZE; } - __copy_from_user(dst[++pg], src + k, len); + if (__copy_from_user(dst[++pg], src + k, len)) + return; } else __copy_from_user((u8 *)dst[pg] + pgoff, src, len); @@ -515,7 +518,8 @@ while (len) { for (voice_num = 0; voice_num < woinst->num_voices; voice_num++) { - __copy_from_user((u8 *)(mem[voice_num].addr[pg]) + pgoff, src, woinst->format.bytespervoicesample); + if (__copy_from_user((u8 *)(mem[voice_num].addr[pg]) + pgoff, src, woinst->format.bytespervoicesample)) + return; src += woinst->format.bytespervoicesample; } diff -Nru a/sound/oss/emu10k1/passthrough.c b/sound/oss/emu10k1/passthrough.c --- a/sound/oss/emu10k1/passthrough.c Sun May 19 03:08:22 2002 +++ b/sound/oss/emu10k1/passthrough.c Sun May 19 03:08:22 2002 @@ -165,12 +165,15 @@ DPD(3, "prepend size %d, prepending %d bytes\n", pt->prepend_size, needed); if (count < needed) { - copy_from_user(pt->buf + pt->prepend_size, buffer, count); + if (copy_from_user(pt->buf + pt->prepend_size, buffer, + count)) + return -EFAULT; pt->prepend_size += count; DPD(3, "prepend size now %d\n", pt->prepend_size); return count; } - copy_from_user(pt->buf + pt->prepend_size, buffer, needed); + if (copy_from_user(pt->buf + pt->prepend_size, buffer, needed)) + return -EFAULT; r = pt_putblock(wave_dev, (u16 *) pt->buf, nonblock); if (r) return r; @@ -181,7 +184,8 @@ blocks_copied = 0; while (blocks > 0) { u16 *bufptr = (u16 *) buffer + (bytes_copied/2); - copy_from_user(pt->buf, bufptr, PT_BLOCKSIZE); + if (copy_from_user(pt->buf, bufptr, PT_BLOCKSIZE)) + return -EFAULT; bufptr = (u16 *) pt->buf; r = pt_putblock(wave_dev, bufptr, nonblock); if (r) { @@ -197,7 +201,8 @@ i = count - bytes_copied; if (i) { pt->prepend_size = i; - copy_from_user(pt->buf, buffer + bytes_copied, i); + if (copy_from_user(pt->buf, buffer + bytes_copied, i)) + return -EFAULT; bytes_copied += i; DPD(3, "filling prepend buffer with %d bytes", i); } diff -Nru a/sound/oss/es1370.c b/sound/oss/es1370.c --- a/sound/oss/es1370.c Sun May 19 03:08:22 2002 +++ b/sound/oss/es1370.c Sun May 19 03:08:22 2002 @@ -1635,7 +1635,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -1653,7 +1655,9 @@ if (s->dma_dac2.mapped) s->dma_dac2.count &= s->dma_dac2.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { @@ -2112,7 +2116,9 @@ if (s->dma_dac1.mapped) s->dma_dac1.count &= s->dma_dac1.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if ((val = prog_dmabuf_dac1(s))) diff -Nru a/sound/oss/es1371.c b/sound/oss/es1371.c --- a/sound/oss/es1371.c Sun May 19 03:08:22 2002 +++ b/sound/oss/es1371.c Sun May 19 03:08:22 2002 @@ -1824,7 +1824,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -1842,8 +1844,9 @@ if (s->dma_dac2.mapped) s->dma_dac2.count &= s->dma_dac2.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); - + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { if ((val = prog_dmabuf_dac2(s))) @@ -2292,7 +2295,9 @@ if (s->dma_dac1.mapped) s->dma_dac1.count &= s->dma_dac1.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if ((val = prog_dmabuf_dac1(s))) diff -Nru a/sound/oss/esssolo1.c b/sound/oss/esssolo1.c --- a/sound/oss/esssolo1.c Sun May 19 03:08:22 2002 +++ b/sound/oss/esssolo1.c Sun May 19 03:08:22 2002 @@ -1468,7 +1468,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -1492,7 +1494,9 @@ cinfo.bytes, cinfo.blocks, cinfo.ptr, s->dma_dac.buforder, s->dma_dac.numfrag, s->dma_dac.fragshift, s->dma_dac.swptr, s->dma_dac.count, s->dma_dac.fragsize, s->dma_dac.dmasize, s->dma_dac.fragsamples); #endif - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/sound/oss/gus_wave.c b/sound/oss/gus_wave.c --- a/sound/oss/gus_wave.c Sun May 19 03:08:22 2002 +++ b/sound/oss/gus_wave.c Sun May 19 03:08:22 2002 @@ -1719,7 +1719,9 @@ * been transferred already. */ - copy_from_user(&((char *) &patch)[offs], &(addr)[offs], sizeof_patch - offs); + if (copy_from_user(&((char *) &patch)[offs], &(addr)[offs], + sizeof_patch - offs)) + return -EFAULT; if (patch.mode & WAVE_ROM) return -EINVAL; @@ -1864,7 +1866,10 @@ * OK, move now. First in and then out. */ - copy_from_user(audio_devs[gus_devnum]->dmap_out->raw_buf, &(addr)[sizeof_patch + src_offs], blk_sz); + if (copy_from_user(audio_devs[gus_devnum]->dmap_out->raw_buf, + &(addr)[sizeof_patch + src_offs], + blk_sz)) + return -EFAULT; save_flags(flags); cli(); diff -Nru a/sound/oss/i810_audio.c b/sound/oss/i810_audio.c --- a/sound/oss/i810_audio.c Sun May 19 03:08:22 2002 +++ b/sound/oss/i810_audio.c Sun May 19 03:08:22 2002 @@ -2020,7 +2020,9 @@ printk("SNDCTL_DSP_GETOPTR %d, %d, %d, %d\n", cinfo.bytes, cinfo.blocks, cinfo.ptr, dmabuf->count); #endif - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETISPACE: if (!(file->f_mode & FMODE_READ)) @@ -2059,8 +2061,9 @@ printk("SNDCTL_DSP_GETIPTR %d, %d, %d, %d\n", cinfo.bytes, cinfo.blocks, cinfo.ptr, dmabuf->count); #endif - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); - + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_NONBLOCK: #ifdef DEBUG printk("SNDCTL_DSP_NONBLOCK\n"); diff -Nru a/sound/oss/ite8172.c b/sound/oss/ite8172.c --- a/sound/oss/ite8172.c Sun May 19 03:08:22 2002 +++ b/sound/oss/ite8172.c Sun May 19 03:08:22 2002 @@ -1427,7 +1427,9 @@ if (count < 0) count = 0; cinfo.blocks = count >> s->dma_adc.fragshift; - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_READ)) @@ -1448,7 +1450,9 @@ if (count < 0) count = 0; cinfo.blocks = count >> s->dma_dac.fragshift; - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) diff -Nru a/sound/oss/maestro.c b/sound/oss/maestro.c --- a/sound/oss/maestro.c Sun May 19 03:08:22 2002 +++ b/sound/oss/maestro.c Sun May 19 03:08:22 2002 @@ -2756,7 +2756,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -2771,7 +2773,9 @@ if (s->dma_dac.mapped) s->dma_dac.count &= s->dma_dac.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/sound/oss/maestro3.c b/sound/oss/maestro3.c --- a/sound/oss/maestro3.c Sun May 19 03:08:22 2002 +++ b/sound/oss/maestro3.c Sun May 19 03:08:22 2002 @@ -1805,7 +1805,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -1818,7 +1820,9 @@ if (s->dma_dac.mapped) s->dma_dac.count &= s->dma_dac.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/sound/oss/midibuf.c b/sound/oss/midibuf.c --- a/sound/oss/midibuf.c Sun May 19 03:08:22 2002 +++ b/sound/oss/midibuf.c Sun May 19 03:08:22 2002 @@ -290,15 +290,15 @@ */ if (file->f_flags & O_NONBLOCK) { - restore_flags(flags); - return -EAGAIN; + c = -EAGAIN; + goto out; } interruptible_sleep_on(&midi_sleeper[dev]); if (signal_pending(current)) { - restore_flags(flags); - return -EINTR; + c = -EINTR; + goto out; } n = SPACE_AVAIL(midi_out_buf[dev]); } @@ -308,11 +308,15 @@ for (i = 0; i < n; i++) { /* BROKE BROKE BROKE - CANT DO THIS WITH CLI !! */ - copy_from_user((char *) &tmp_data, &(buf)[c], 1); + if (copy_from_user((char *) &tmp_data, &(buf)[c], 1)) { + c = -EFAULT; + goto out; + } QUEUE_BYTE(midi_out_buf[dev], tmp_data); c++; } } +out: restore_flags(flags); return c; } @@ -333,8 +337,8 @@ * No data yet, wait */ if (file->f_flags & O_NONBLOCK) { - restore_flags(flags); - return -EAGAIN; + c = -EAGAIN; + goto out; } interruptible_sleep_on_timeout(&input_sleeper[dev], parms[dev].prech_timeout); @@ -357,10 +361,14 @@ REMOVE_BYTE(midi_in_buf[dev], tmp_data); fixit = (char *) &tmp_data; /* BROKE BROKE BROKE */ - copy_to_user(&(buf)[c], fixit, 1); + if (copy_to_user(&(buf)[c], fixit, 1)) { + c = -EFAULT; + goto out; + } c++; } } +out: restore_flags(flags); return c; } diff -Nru a/sound/oss/msnd_pinnacle.c b/sound/oss/msnd_pinnacle.c --- a/sound/oss/msnd_pinnacle.c Sun May 19 03:08:22 2002 +++ b/sound/oss/msnd_pinnacle.c Sun May 19 03:08:22 2002 @@ -564,11 +564,15 @@ mixer_info info; set_mixer_info(); info.modify_counter = dev.mixer_mod_count; - return copy_to_user((void *)arg, &info, sizeof(info)); + if (copy_to_user((void *)arg, &info, sizeof(info))) + return -EFAULT; + return 0; } else if (cmd == SOUND_OLD_MIXER_INFO) { _old_mixer_info info; set_mixer_info(); - return copy_to_user((void *)arg, &info, sizeof(info)); + if (copy_to_user((void *)arg, &info, sizeof(info))) + return -EFAULT; + return 0; } else if (cmd == SOUND_MIXER_PRIVATE1) { dev.nresets = 0; dsp_full_reset(); diff -Nru a/sound/oss/rme96xx.c b/sound/oss/rme96xx.c --- a/sound/oss/rme96xx.c Sun May 19 03:08:22 2002 +++ b/sound/oss/rme96xx.c Sun May 19 03:08:22 2002 @@ -1083,7 +1083,9 @@ dma->readptr &= s->fragsize<<1; spin_unlock_irqrestore(&s->lock,flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_READ)) @@ -1100,7 +1102,9 @@ if (dma->mmapped) dma->writeptr &= s->fragsize<<1; spin_unlock_irqrestore(&s->lock,flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: return put_user(s->fragsize, (int *)arg); @@ -1520,7 +1524,8 @@ VALIDATE_STATE(s); if (cmd == SOUND_MIXER_PRIVATE1) { rme_mixer mixer; - copy_from_user(&mixer,(void*)arg,sizeof(mixer)); + if (copy_from_user(&mixer,(void*)arg,sizeof(mixer))) + return -EFAULT; if (file->f_mode & FMODE_WRITE) { s->dma[mixer.devnr].outoffset = mixer.o_offset; @@ -1537,7 +1542,8 @@ } if (cmd == SOUND_MIXER_PRIVATE3) { u32 control; - copy_from_user(&control,(void*)arg,sizeof(control)); + if (copy_from_user(&control,(void*)arg,sizeof(control))) + return -EFAULT; if (file->f_mode & FMODE_WRITE) { s->control_register = control; writel(control,s->iobase + RME96xx_control_register); diff -Nru a/sound/oss/sb_audio.c b/sound/oss/sb_audio.c --- a/sound/oss/sb_audio.c Sun May 19 03:08:22 2002 +++ b/sound/oss/sb_audio.c Sun May 19 03:08:22 2002 @@ -849,7 +849,9 @@ /* if not duplex no conversion */ if (!devc->fullduplex) { - copy_from_user (localbuf + localoffs, userbuf + useroffs, len); + if (copy_from_user(localbuf + localoffs, + userbuf + useroffs, len)) + return -EFAULT; *used = len; *returned = len; } @@ -869,9 +871,10 @@ { locallen = (c >= LBUFCOPYSIZE ? LBUFCOPYSIZE : c); /* << 1 in order to get 16 bit samples */ - copy_from_user (lbuf16, - userbuf+useroffs + (p << 1), - locallen << 1); + if (copy_from_user(lbuf16, + userbuf + useroffs + (p << 1), + locallen << 1)) + return -EFAULT; for (i = 0; i < locallen; i++) { buf8[p+i] = ~((lbuf16[i] >> 8) & 0xff) ^ 0x80; @@ -898,9 +901,10 @@ while (c) { locallen = (c >= LBUFCOPYSIZE ? LBUFCOPYSIZE : c); - copy_from_user (lbuf8, - userbuf+useroffs + p, - locallen); + if (copy_from_user(lbuf8, + userbuf+useroffs + p, + locallen)) + return -EFAULT; for (i = 0; i < locallen; i++) { buf16[p+i] = (~lbuf8[i] ^ 0x80) << 8; diff -Nru a/sound/oss/sequencer.c b/sound/oss/sequencer.c --- a/sound/oss/sequencer.c Sun May 19 03:08:22 2002 +++ b/sound/oss/sequencer.c Sun May 19 03:08:22 2002 @@ -116,13 +116,15 @@ while (iqlen && c >= ev_len) { char *fixit = (char *) &iqueue[iqhead * IEV_SZ]; - copy_to_user(&(buf)[p], fixit, ev_len); + if (copy_to_user(&(buf)[p], fixit, ev_len)) + goto out; p += ev_len; c -= ev_len; iqhead = (iqhead + 1) % SEQ_MAX_QUEUE; iqlen--; } +out: restore_flags(flags); return count - c; } @@ -226,7 +228,8 @@ while (c >= 4) { - copy_from_user((char *) event_rec, &(buf)[p], 4); + if (copy_from_user((char *) event_rec, &(buf)[p], 4)) + goto out; ev_code = event_rec[0]; if (ev_code == SEQ_FULLSIZE) @@ -262,7 +265,9 @@ seq_startplay(); return count - c; } - copy_from_user((char *) &event_rec[4], &(buf)[p + 4], 4); + if (copy_from_user((char *)&event_rec[4], + &(buf)[p + 4], 4)) + goto out; } else @@ -320,7 +325,7 @@ if (!seq_playing) seq_startplay(); - +out: return count; } diff -Nru a/sound/oss/sonicvibes.c b/sound/oss/sonicvibes.c --- a/sound/oss/sonicvibes.c Sun May 19 03:08:22 2002 +++ b/sound/oss/sonicvibes.c Sun May 19 03:08:22 2002 @@ -1802,7 +1802,9 @@ if (s->dma_adc.mapped) s->dma_adc.count &= s->dma_adc.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -1820,7 +1822,9 @@ if (s->dma_dac.mapped) s->dma_dac.count &= s->dma_dac.fragsize-1; spin_unlock_irqrestore(&s->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/sound/oss/trident.c b/sound/oss/trident.c --- a/sound/oss/trident.c Sun May 19 03:08:22 2002 +++ b/sound/oss/trident.c Sun May 19 03:08:22 2002 @@ -2447,7 +2447,8 @@ if (dmabuf->mapped) dmabuf->count &= dmabuf->fragsize-1; spin_unlock_irqrestore(&state->card->lock, flags); - ret = copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + ret = copy_to_user((void *)arg, &cinfo, sizeof(cinfo)) ? + -EFAULT : 0; break; case SNDCTL_DSP_GETOPTR: diff -Nru a/sound/oss/via82cxxx_audio.c b/sound/oss/via82cxxx_audio.c --- a/sound/oss/via82cxxx_audio.c Sun May 19 03:08:22 2002 +++ b/sound/oss/via82cxxx_audio.c Sun May 19 03:08:22 2002 @@ -2526,7 +2526,7 @@ info.fragments, info.bytes); - return copy_to_user (arg, &info, sizeof (info)); + return copy_to_user(arg, &info, sizeof (info)) ? -EFAULT : 0; } @@ -2570,7 +2570,7 @@ info.blocks, info.ptr); - return copy_to_user (arg, &info, sizeof (info)); + return copy_to_user(arg, &info, sizeof (info)) ? -EFAULT : 0; } diff -Nru a/sound/oss/vwsnd.c b/sound/oss/vwsnd.c --- a/sound/oss/vwsnd.c Sun May 19 03:08:22 2002 +++ b/sound/oss/vwsnd.c Sun May 19 03:08:22 2002 @@ -2303,7 +2303,8 @@ if (nb > count) nb = count; DBGPV("nb = %d\n", nb); - copy_to_user(buffer, rport->swbuf + rport->swb_u_idx, nb); + if (copy_to_user(buffer, rport->swbuf + rport->swb_u_idx, nb)) + return -EFAULT; (void) swb_inc_u(rport, nb); buffer += nb; count -= nb; @@ -2377,7 +2378,8 @@ if (nb > count) nb = count; DBGPV("nb = %d\n", nb); - copy_from_user(wport->swbuf + wport->swb_u_idx, buffer, nb); + if (copy_from_user(wport->swbuf + wport->swb_u_idx, buffer, nb)) + return -EFAULT; pcm_output(devc, 0, nb); buffer += nb; count -= nb; @@ -2650,7 +2652,9 @@ DBGXV("SNDCTL_DSP_GETOSPACE returns { %d %d %d %d }\n", buf_info.fragments, buf_info.fragstotal, buf_info.fragsize, buf_info.bytes); - return copy_to_user((void *) arg, &buf_info, sizeof buf_info); + if (copy_to_user((void *) arg, &buf_info, sizeof buf_info)) + return -EFAULT; + return 0; case SNDCTL_DSP_GETISPACE: /* _SIOR ('P',13, audio_buf_info) */ DBGX("SNDCTL_DSP_GETISPACE\n"); @@ -2667,7 +2671,9 @@ DBGX("SNDCTL_DSP_GETISPACE returns { %d %d %d %d }\n", buf_info.fragments, buf_info.fragstotal, buf_info.fragsize, buf_info.bytes); - return copy_to_user((void *) arg, &buf_info, sizeof buf_info); + if (copy_to_user((void *) arg, &buf_info, sizeof buf_info)) + return -EFAULT; + return 0; case SNDCTL_DSP_NONBLOCK: /* _SIO ('P',14) */ DBGX("SNDCTL_DSP_NONBLOCK\n"); @@ -2725,7 +2731,9 @@ rport->frag_count = 0; } spin_unlock_irqrestore(&rport->lock, flags); - return copy_to_user((void *) arg, &info, sizeof info); + if (copy_to_user((void *) arg, &info, sizeof info)) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: /* _SIOR ('P',18, count_info) */ DBGX("SNDCTL_DSP_GETOPTR\n"); @@ -2747,7 +2755,9 @@ wport->frag_count = 0; } spin_unlock_irqrestore(&wport->lock, flags); - return copy_to_user((void *) arg, &info, sizeof info); + if (copy_to_user((void *) arg, &info, sizeof info)) + return -EFAULT; + return 0; case SNDCTL_DSP_GETODELAY: /* _SIOR ('P', 23, int) */ DBGX("SNDCTL_DSP_GETODELAY\n"); diff -Nru a/sound/oss/wavfront.c b/sound/oss/wavfront.c --- a/sound/oss/wavfront.c Sun May 19 03:08:22 2002 +++ b/sound/oss/wavfront.c Sun May 19 03:08:22 2002 @@ -1525,8 +1525,9 @@ /* Copy in the header of the GUS patch */ sizeof_patch = (long) &guspatch.data[0] - (long) &guspatch; - copy_from_user (&((char *) &guspatch)[offs], - &(addr)[offs], sizeof_patch - offs); + if (copy_from_user(&((char *) &guspatch)[offs], + &(addr)[offs], sizeof_patch - offs)) + return -EFAULT; if ((i = wavefront_find_free_patch ()) == -1) { return -EBUSY; @@ -1662,7 +1663,7 @@ if (copy_from_user (&header, addr, sizeof(wavefront_patch_info) - sizeof(wavefront_any))) { printk (KERN_WARNING LOGNAME "bad address for load patch.\n"); - return -(EINVAL); + return -EFAULT; } DPRINT (WF_DEBUG_LOAD_PATCH, "download " @@ -1676,47 +1677,53 @@ switch (header.subkey) { case WF_ST_SAMPLE: /* sample or sample_header, based on patch->size */ - copy_from_user ((unsigned char *) &header.hdr.s, - (unsigned char *) header.hdrptr, - sizeof (wavefront_sample)); + if (copy_from_user((unsigned char *) &header.hdr.s, + (unsigned char *) header.hdrptr, + sizeof (wavefront_sample))) + return -EFAULT; return wavefront_send_sample (&header, header.dataptr, 0); case WF_ST_MULTISAMPLE: - copy_from_user ((unsigned char *) &header.hdr.s, - (unsigned char *) header.hdrptr, - sizeof (wavefront_multisample)); + if (copy_from_user((unsigned char *) &header.hdr.s, + (unsigned char *) header.hdrptr, + sizeof(wavefront_multisample))) + return -EFAULT; return wavefront_send_multisample (&header); case WF_ST_ALIAS: - copy_from_user ((unsigned char *) &header.hdr.a, - (unsigned char *) header.hdrptr, - sizeof (wavefront_alias)); + if (copy_from_user((unsigned char *) &header.hdr.a, + (unsigned char *) header.hdrptr, + sizeof (wavefront_alias))) + return -EFAULT; return wavefront_send_alias (&header); case WF_ST_DRUM: - copy_from_user ((unsigned char *) &header.hdr.d, - (unsigned char *) header.hdrptr, - sizeof (wavefront_drum)); + if (copy_from_user((unsigned char *) &header.hdr.d, + (unsigned char *) header.hdrptr, + sizeof (wavefront_drum))) + return -EFAULT; return wavefront_send_drum (&header); case WF_ST_PATCH: - copy_from_user ((unsigned char *) &header.hdr.p, - (unsigned char *) header.hdrptr, - sizeof (wavefront_patch)); + if (copy_from_user((unsigned char *) &header.hdr.p, + (unsigned char *) header.hdrptr, + sizeof (wavefront_patch))) + return -EFAULT; return wavefront_send_patch (&header); case WF_ST_PROGRAM: - copy_from_user ((unsigned char *) &header.hdr.pr, - (unsigned char *) header.hdrptr, - sizeof (wavefront_program)); + if (copy_from_user((unsigned char *) &header.hdr.pr, + (unsigned char *) header.hdrptr, + sizeof (wavefront_program))) + return -EFAULT; return wavefront_send_program (&header); @@ -1931,10 +1938,12 @@ switch (cmd) { case WFCTL_WFCMD: - copy_from_user (&wc, (void *) arg, sizeof (wc)); + if (copy_from_user(&wc, (void *) arg, sizeof (wc))) + return -EFAULT; if ((err = wavefront_synth_control (cmd, &wc)) == 0) { - copy_to_user ((void *) arg, &wc, sizeof (wc)); + if (copy_to_user ((void *) arg, &wc, sizeof (wc))) + return -EFAULT; } return err; @@ -2995,8 +3004,10 @@ "> 255 bytes to FX\n"); return -(EINVAL); } - copy_from_user (page_data, (unsigned char *) r->data[3], - r->data[2]); + if (copy_from_user(page_data, + (unsigned char *)r->data[3], + r->data[2])) + return -EFAULT; pd = page_data; } ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-18 23:54 ` Arnaldo Carvalho de Melo 2002-05-19 0:14 ` [BKPATCH] OSS: " Arnaldo Carvalho de Melo @ 2002-05-19 0:19 ` Arnaldo Carvalho de Melo 2002-05-19 1:16 ` Arnaldo Carvalho de Melo 2002-05-19 6:30 ` Kai Germaschewski 1 sibling, 2 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 0:19 UTC (permalink / raw) To: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 08:54:19PM -0300, Arnaldo C. Melo escreveu: > Em Sat, May 18, 2002 at 07:55:35PM -0300, Arnaldo C. Melo escreveu: > > Heads up: I'm finishing a bk changeset for intermezzo, will be submitting > > to Linus in some minutes. > > Second heads up: > > OSS will be on its way to Linus in some minutes... Third heads up: ISDN will be on its way to Linus in some minutes... - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 0:19 ` Arnaldo Carvalho de Melo @ 2002-05-19 1:16 ` Arnaldo Carvalho de Melo 2002-05-19 1:38 ` [BKPATCH] USB: " Arnaldo Carvalho de Melo 2002-05-19 6:30 ` Kai Germaschewski 1 sibling, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 1:16 UTC (permalink / raw) To: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 09:19:15PM -0300, Arnaldo C. Melo escreveu: > Em Sat, May 18, 2002 at 08:54:19PM -0300, Arnaldo C. Melo escreveu: > > Em Sat, May 18, 2002 at 07:55:35PM -0300, Arnaldo C. Melo escreveu: > > > Heads up: I'm finishing a bk changeset for intermezzo, will be submitting > > > to Linus in some minutes. > > > > Second heads up: > > > > OSS will be on its way to Linus in some minutes... > > Third heads up: > > ISDN will be on its way to Linus in some minutes... 4th heads up: USB will be on its way to Linus in some minutes, I already talked with Greg :) - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
* [BKPATCH] USB: Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 1:16 ` Arnaldo Carvalho de Melo @ 2002-05-19 1:38 ` Arnaldo Carvalho de Melo 2002-05-20 6:07 ` Greg KH 0 siblings, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 1:38 UTC (permalink / raw) To: Linus Torvalds, Greg Kroah-Hartman Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 10:16:01PM -0300, Arnaldo C. Melo escreveu: > 4th heads up: > > USB will be on its way to Linus in some minutes, I already talked with Greg :) Linus, Greg, Please consider pulling from: http://kernel-acme.bkbits.net:8080/usb-copy_tofrom_user-2.5 - Arnaldo # This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.545 -> 1.546 # drivers/usb/class/cdc-acm.c 1.16 -> 1.17 # drivers/usb/host/uhci-debug.c 1.1 -> 1.2 # drivers/usb/class/bluetty.c 1.21 -> 1.22 # drivers/usb/serial/safe_serial.c 1.1 -> 1.2 # drivers/usb/serial/ipaq.c 1.8 -> 1.9 # drivers/usb/misc/auerswald.c 1.13 -> 1.14 # drivers/usb/input/hiddev.c 1.12 -> 1.13 # drivers/usb/class/audio.c 1.18 -> 1.19 # drivers/usb/media/dabusb.c 1.16 -> 1.17 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/05/19 acme@conectiva.com.br 1.546 # drivers/usr/*.c # # - fix copy_{to,from}_user error handling (thanks to Rusty for pointing this out) # -------------------------------------------- # diff -Nru a/drivers/usb/class/audio.c b/drivers/usb/class/audio.c --- a/drivers/usb/class/audio.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/class/audio.c Sun May 19 04:33:19 2002 @@ -2542,7 +2542,9 @@ if (as->usbin.dma.mapped) as->usbin.dma.count &= as->usbin.dma.fragsize-1; spin_unlock_irqrestore(&as->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETOPTR: if (!(file->f_mode & FMODE_WRITE)) @@ -2554,7 +2556,9 @@ if (as->usbout.dma.mapped) as->usbout.dma.count &= as->usbout.dma.fragsize-1; spin_unlock_irqrestore(&as->lock, flags); - return copy_to_user((void *)arg, &cinfo, sizeof(cinfo)); + if (copy_to_user((void *)arg, &cinfo, sizeof(cinfo))) + return -EFAULT; + return 0; case SNDCTL_DSP_GETBLKSIZE: if (file->f_mode & FMODE_WRITE) { diff -Nru a/drivers/usb/class/bluetty.c b/drivers/usb/class/bluetty.c --- a/drivers/usb/class/bluetty.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/class/bluetty.c Sun May 19 04:33:19 2002 @@ -490,7 +490,10 @@ retval = -ENOMEM; goto exit; } - copy_from_user (temp_buffer, buf, count); + if (copy_from_user (temp_buffer, buf, count)) { + retval = -EFAULT; + goto exit; + } current_buffer = temp_buffer; } else { current_buffer = buf; diff -Nru a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c --- a/drivers/usb/class/cdc-acm.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/class/cdc-acm.c Sun May 19 04:33:19 2002 @@ -367,9 +367,10 @@ count = (count > acm->writesize) ? acm->writesize : count; - if (from_user) - copy_from_user(acm->writeurb->transfer_buffer, buf, count); - else + if (from_user) { + if (copy_from_user(acm->writeurb->transfer_buffer, buf, count)) + return -EFAULT; + } else memcpy(acm->writeurb->transfer_buffer, buf, count); acm->writeurb->transfer_buffer_length = count; diff -Nru a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c --- a/drivers/usb/host/uhci-debug.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/host/uhci-debug.c Sun May 19 04:33:19 2002 @@ -552,7 +552,8 @@ if (!access_ok(VERIFY_WRITE, buf, nbytes)) return -EINVAL; - copy_to_user(buf, up->data + pos, nbytes); + if (copy_to_user(buf, up->data + pos, nbytes)) + return -EFAULT; *ppos += nbytes; diff -Nru a/drivers/usb/input/hiddev.c b/drivers/usb/input/hiddev.c --- a/drivers/usb/input/hiddev.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/input/hiddev.c Sun May 19 04:33:19 2002 @@ -389,7 +389,9 @@ dinfo.product = dev->descriptor.idProduct; dinfo.version = dev->descriptor.bcdDevice; dinfo.num_applications = hid->maxapplication; - return copy_to_user((void *) arg, &dinfo, sizeof(dinfo)); + if (copy_to_user((void *) arg, &dinfo, sizeof(dinfo))) + return -EFAULT; + return 0; } case HIDIOCGFLAG: @@ -480,7 +482,9 @@ rinfo.num_fields = report->maxfield; - return copy_to_user((void *) arg, &rinfo, sizeof(rinfo)); + if (copy_to_user((void *) arg, &rinfo, sizeof(rinfo))) + return -EFAULT; + return 0; case HIDIOCGFIELDINFO: { @@ -512,7 +516,9 @@ finfo.unit_exponent = field->unit_exponent; finfo.unit = field->unit; - return copy_to_user((void *) arg, &finfo, sizeof(finfo)); + if (copy_to_user((void *) arg, &finfo, sizeof(finfo))) + return -EFAULT; + return 0; } case HIDIOCGUCODE: @@ -533,7 +539,9 @@ uref.usage_code = field->usage[uref.usage_index].hid; - return copy_to_user((void *) arg, &uref, sizeof(uref)); + if (copy_to_user((void *) arg, &uref, sizeof(uref))) + return -EFAULT; + return 0; case HIDIOCGUSAGE: case HIDIOCSUSAGE: @@ -564,7 +572,9 @@ if (cmd == HIDIOCGUSAGE) { uref.value = field->value[uref.usage_index]; - return copy_to_user((void *) arg, &uref, sizeof(uref)); + if (copy_to_user((void *) arg, &uref, sizeof(uref))) + return -EFAULT; + return 0; } else { field->value[uref.usage_index] = uref.value; } diff -Nru a/drivers/usb/media/dabusb.c b/drivers/usb/media/dabusb.c --- a/drivers/usb/media/dabusb.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/media/dabusb.c Sun May 19 04:33:19 2002 @@ -680,7 +680,9 @@ ret=dabusb_bulk (s, pbulk); if(ret==0) - ret = copy_to_user ((void *) arg, pbulk, sizeof (bulk_transfer_t)); + if (copy_to_user((void *)arg, pbulk, + sizeof(bulk_transfer_t))) + ret = -EFAULT; kfree (pbulk); break; diff -Nru a/drivers/usb/misc/auerswald.c b/drivers/usb/misc/auerswald.c --- a/drivers/usb/misc/auerswald.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/misc/auerswald.c Sun May 19 04:33:19 2002 @@ -1553,7 +1553,7 @@ if (u > devinfo.bsize) { u = devinfo.bsize; } - ret = copy_to_user(devinfo.buf, cp->dev_desc, u); + ret = copy_to_user(devinfo.buf, cp->dev_desc, u) ? -EFAULT : 0; break; /* get the max. string descriptor length */ @@ -1803,7 +1803,7 @@ wake_up (&cp->bufferwait); up (&cp->mutex); up (&ccp->mutex); - return -EIO; + return -EFAULT; } /* set the header byte */ diff -Nru a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c --- a/drivers/usb/serial/ipaq.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/serial/ipaq.c Sun May 19 04:33:19 2002 @@ -353,7 +353,8 @@ } if (from_user) { - copy_from_user(pkt->data, buf, count); + if (copy_from_user(pkt->data, buf, count)) + return -EFAULT; } else { memcpy(pkt->data, buf, count); } diff -Nru a/drivers/usb/serial/safe_serial.c b/drivers/usb/serial/safe_serial.c --- a/drivers/usb/serial/safe_serial.c Sun May 19 04:33:19 2002 +++ b/drivers/usb/serial/safe_serial.c Sun May 19 04:33:19 2002 @@ -319,7 +319,8 @@ memset (data, '0', packet_length); if (from_user) { - copy_from_user (data, buf, count); + if (copy_from_user (data, buf, count)) + return -EFAULT; } else { memcpy (data, buf, count); } ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [BKPATCH] USB: Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 1:38 ` [BKPATCH] USB: " Arnaldo Carvalho de Melo @ 2002-05-20 6:07 ` Greg KH 0 siblings, 0 replies; 30+ messages in thread From: Greg KH @ 2002-05-20 6:07 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, linux-kernel, kernel-janitor-discuss On Sat, May 18, 2002 at 10:38:08PM -0300, Arnaldo Carvalho de Melo wrote: > Em Sat, May 18, 2002 at 10:16:01PM -0300, Arnaldo C. Melo escreveu: > > 4th heads up: > > > > USB will be on its way to Linus in some minutes, I already talked with Greg :) > > Linus, Greg, > > Please consider pulling from: > > http://kernel-acme.bkbits.net:8080/usb-copy_tofrom_user-2.5 Looks good, and it looks like Linus already pulled this. Thanks for fixing this up for the USB (and other subsystems.) greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 0:19 ` Arnaldo Carvalho de Melo 2002-05-19 1:16 ` Arnaldo Carvalho de Melo @ 2002-05-19 6:30 ` Kai Germaschewski 2002-05-19 0:45 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 30+ messages in thread From: Kai Germaschewski @ 2002-05-19 6:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss On Sat, 18 May 2002, Arnaldo Carvalho de Melo wrote: > ISDN will be on its way to Linus in some minutes... I surely remember fixing that some months back already. Apparently I lost the patch somewhere on the way ;( Anyway, I'll be happy to do this for ISDN, but if you have it already done, go ahead (and CC me when you submit it, please). --Kai ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 6:30 ` Kai Germaschewski @ 2002-05-19 0:45 ` Arnaldo Carvalho de Melo 2002-05-19 1:07 ` [BKPATCH] ISDN: " Arnaldo Carvalho de Melo 0 siblings, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 0:45 UTC (permalink / raw) To: Kai Germaschewski; +Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sun, May 19, 2002 at 01:30:34AM -0500, Kai Germaschewski escreveu: > On Sat, 18 May 2002, Arnaldo Carvalho de Melo wrote: > > > ISDN will be on its way to Linus in some minutes... > > I surely remember fixing that some months back already. Apparently I lost > the patch somewhere on the way ;( > > Anyway, I'll be happy to do this for ISDN, but if you have it already > done, go ahead (and CC me when you submit it, please). Of course, I'm finishing it in some minutes. - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
* [BKPATCH] ISDN: Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 0:45 ` Arnaldo Carvalho de Melo @ 2002-05-19 1:07 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 1:07 UTC (permalink / raw) To: Linus Torvalds Cc: Kai Germaschewski, Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sat, May 18, 2002 at 09:45:58PM -0300, Arnaldo C. Melo escreveu: > Em Sun, May 19, 2002 at 01:30:34AM -0500, Kai Germaschewski escreveu: > > On Sat, 18 May 2002, Arnaldo Carvalho de Melo wrote: > > > > > ISDN will be on its way to Linus in some minutes... > > > > I surely remember fixing that some months back already. Apparently I lost > > the patch somewhere on the way ;( > > > > Anyway, I'll be happy to do this for ISDN, but if you have it already > > done, go ahead (and CC me when you submit it, please). > > Of course, I'm finishing it in some minutes. Here it is, Linus/Kai, please consider pulling it from: http://kernel-acme.bkbits.net:8080/isdn-copy_tofrom_user-2.5 - Arnaldo # This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.544 -> 1.545 # drivers/isdn/isdnloop/isdnloop.c 1.8 -> 1.9 # drivers/isdn/hardware/avm/c4.c 1.25 -> 1.26 # drivers/isdn/eicon/eicon_mod.c 1.7 -> 1.8 # drivers/isdn/tpam/tpam_commands.c 1.3 -> 1.4 # drivers/isdn/i4l/isdn_tty.c 1.11 -> 1.12 # drivers/isdn/act2000/module.c 1.4 -> 1.5 # drivers/isdn/sc/command.c 1.3 -> 1.4 # drivers/isdn/hardware/avm/b1.c 1.16 -> 1.17 # drivers/isdn/hisax/isar.c 1.8 -> 1.9 # drivers/isdn/icn/icn.c 1.8 -> 1.9 # drivers/isdn/sc/ioctl.c 1.2 -> 1.3 # drivers/isdn/i4l/isdn_ppp.c 1.16 -> 1.17 # drivers/isdn/capi/kcapi.c 1.30 -> 1.31 # drivers/isdn/hisax/config.c 1.16 -> 1.17 # drivers/isdn/divert/divert_procfs.c 1.8 -> 1.9 # drivers/isdn/capi/capi.c 1.29 -> 1.30 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/05/19 acme@conectiva.com.br 1.545 # drivers/isdn/*.c # # - fix copy_{to,from}_user error handling (thanks to Rusty for pointing this out) # -------------------------------------------- # diff -Nru a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c --- a/drivers/isdn/act2000/module.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/act2000/module.c Sun May 19 03:59:20 2002 @@ -283,16 +283,18 @@ actcapi_manufacturer_req_net(card); return 0; case ACT2000_IOCTL_SETMSN: - if ((ret = copy_from_user(tmp, (char *)a, sizeof(tmp)))) - return ret; + if (copy_from_user(tmp, (char *)a, + sizeof(tmp))) + return -EFAULT; if ((ret = act2000_set_msn(card, tmp))) return ret; if (card->flags & ACT2000_FLAGS_RUNNING) return(actcapi_manufacturer_req_msn(card)); return 0; case ACT2000_IOCTL_ADDCARD: - if ((ret = copy_from_user(&cdef, (char *)a, sizeof(cdef)))) - return ret; + if (copy_from_user(&cdef, (char *)a, + sizeof(cdef))) + return -EFAULT; if (act2000_addcard(cdef.bus, cdef.port, cdef.irq, cdef.id)) return -EIO; return 0; diff -Nru a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c --- a/drivers/isdn/capi/capi.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/capi/capi.c Sun May 19 03:59:20 2002 @@ -673,10 +673,9 @@ skb_queue_head(&cdev->recvqueue, skb); return -EMSGSIZE; } - retval = copy_to_user(buf, skb->data, skb->len); - if (retval) { + if (copy_to_user(buf, skb->data, skb->len)) { skb_queue_head(&cdev->recvqueue, skb); - return retval; + return -EFAULT; } copied = skb->len; @@ -703,7 +702,7 @@ if (!skb) return -ENOMEM; - if ((retval = copy_from_user(skb_put(skb, count), buf, count))) { + if (copy_from_user(skb_put(skb, count), buf, count)) { kfree_skb(skb); return -EFAULT; } @@ -782,45 +781,36 @@ case CAPI_GET_VERSION: { - retval = copy_from_user((void *) &data.contr, + if (copy_from_user((void *) &data.contr, (void *) arg, - sizeof(data.contr)); - if (retval) + sizeof(data.contr))) return -EFAULT; cdev->errcode = capi20_get_version(data.contr, &data.version); if (cdev->errcode) return -EIO; - retval = copy_to_user((void *) arg, - (void *) &data.version, - sizeof(data.version)); - if (retval) + if (copy_to_user((void *)arg, (void *)&data.version, + sizeof(data.version))) return -EFAULT; } return 0; case CAPI_GET_SERIAL: { - retval = copy_from_user((void *) &data.contr, - (void *) arg, - sizeof(data.contr)); - if (retval) + if (copy_from_user((void *)&data.contr, (void *)arg, + sizeof(data.contr))) return -EFAULT; cdev->errcode = capi20_get_serial (data.contr, data.serial); if (cdev->errcode) return -EIO; - retval = copy_to_user((void *) arg, - (void *) data.serial, - sizeof(data.serial)); - if (retval) + if (copy_to_user((void *)arg, (void *)data.serial, + sizeof(data.serial))) return -EFAULT; } return 0; case CAPI_GET_PROFILE: { - retval = copy_from_user((void *) &data.contr, - (void *) arg, - sizeof(data.contr)); - if (retval) + if (copy_from_user((void *)&data.contr, (void *)arg, + sizeof(data.contr))) return -EFAULT; if (data.contr == 0) { @@ -848,18 +838,15 @@ case CAPI_GET_MANUFACTURER: { - retval = copy_from_user((void *) &data.contr, - (void *) arg, - sizeof(data.contr)); - if (retval) + if (copy_from_user((void *)&data.contr, (void *)arg, + sizeof(data.contr))) return -EFAULT; cdev->errcode = capi20_get_manufacturer(data.contr, data.manufacturer); if (cdev->errcode) return -EIO; - retval = copy_to_user((void *) arg, (void *) data.manufacturer, - sizeof(data.manufacturer)); - if (retval) + if (copy_to_user((void *)arg, (void *)data.manufacturer, + sizeof(data.manufacturer))) return -EFAULT; } @@ -868,10 +855,8 @@ data.errcode = cdev->errcode; cdev->errcode = CAPI_NOERROR; if (arg) { - retval = copy_to_user((void *) arg, - (void *) &data.errcode, - sizeof(data.errcode)); - if (retval) + if (copy_to_user((void *)arg, (void *)&data.errcode, + sizeof(data.errcode))) return -EFAULT; } return data.errcode; @@ -886,9 +871,8 @@ struct capi_manufacturer_cmd mcmd; if (!capable(CAP_SYS_ADMIN)) return -EPERM; - retval = copy_from_user((void *) &mcmd, (void *) arg, - sizeof(mcmd)); - if (retval) + if (copy_from_user((void *)&mcmd, (void *)arg, + sizeof(mcmd))) return -EFAULT; return capi20_manufacturer(mcmd.cmd, mcmd.data); } @@ -898,10 +882,8 @@ case CAPI_CLR_FLAGS: { unsigned userflags; - retval = copy_from_user((void *) &userflags, - (void *) arg, - sizeof(userflags)); - if (retval) + if (copy_from_user((void *)&userflags, (void *)arg, + sizeof(userflags))) return -EFAULT; if (cmd == CAPI_SET_FLAGS) cdev->userflags |= userflags; @@ -911,13 +893,9 @@ return 0; case CAPI_GET_FLAGS: - { - retval = copy_to_user((void *) arg, - (void *) &cdev->userflags, - sizeof(cdev->userflags)); - if (retval) - return -EFAULT; - } + if (copy_to_user((void *)arg, (void *)&cdev->userflags, + sizeof(cdev->userflags))) + return -EFAULT; return 0; case CAPI_NCCI_OPENCOUNT: @@ -928,10 +906,8 @@ #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */ unsigned ncci; int count = 0; - retval = copy_from_user((void *) &ncci, - (void *) arg, - sizeof(ncci)); - if (retval) + if (copy_from_user((void *)&ncci, (void *)arg, + sizeof(ncci))) return -EFAULT; nccip = capincci_find(cdev, (u32) ncci); if (!nccip) @@ -951,10 +927,8 @@ struct capincci *nccip; struct capiminor *mp; unsigned ncci; - retval = copy_from_user((void *) &ncci, - (void *) arg, - sizeof(ncci)); - if (retval) + if (copy_from_user((void *)&ncci, (void *)arg, + sizeof(ncci))) return -EFAULT; nccip = capincci_find(cdev, (u32) ncci); if (!nccip || (mp = nccip->minorp) == 0) diff -Nru a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c --- a/drivers/isdn/capi/kcapi.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/capi/kcapi.c Sun May 19 03:59:20 2002 @@ -1060,15 +1060,15 @@ case AVMB1_LOAD_AND_CONFIG: if (cmd == AVMB1_LOAD) { - if ((retval = copy_from_user((void *) &ldef, data, - sizeof(avmb1_loaddef)))) - return retval; + if (copy_from_user((void *)&ldef, data, + sizeof(avmb1_loaddef))) + return -EFAULT; ldef.t4config.len = 0; ldef.t4config.data = 0; } else { - if ((retval = copy_from_user((void *) &ldef, data, - sizeof(avmb1_loadandconfigdef)))) - return retval; + if (copy_from_user((void *)&ldef, data, + sizeof(avmb1_loadandconfigdef))) + return -EFAULT; } card = get_capi_ctr_by_nr(ldef.contr); card = capi_ctr_get(card); @@ -1123,9 +1123,8 @@ return 0; case AVMB1_RESETCARD: - if ((retval = copy_from_user((void *) &rdef, data, - sizeof(avmb1_resetdef)))) - return retval; + if (copy_from_user((void *)&rdef, data, sizeof(avmb1_resetdef))) + return -EFAULT; card = get_capi_ctr_by_nr(rdef.contr); if (!card) return -ESRCH; @@ -1146,9 +1145,8 @@ return 0; case AVMB1_GET_CARDINFO: - if ((retval = copy_from_user((void *) &gdef, data, - sizeof(avmb1_getdef)))) - return retval; + if (copy_from_user((void *)&gdef, data, sizeof(avmb1_getdef))) + return -EFAULT; card = get_capi_ctr_by_nr(gdef.contr); if (!card) @@ -1159,9 +1157,8 @@ gdef.cardtype = AVM_CARDTYPE_T1; else gdef.cardtype = AVM_CARDTYPE_B1; - if ((retval = copy_to_user(data, (void *) &gdef, - sizeof(avmb1_getdef)))) - return retval; + if (copy_to_user(data, (void *)&gdef, sizeof(avmb1_getdef))) + return -EFAULT; return 0; } @@ -1187,9 +1184,8 @@ { kcapi_flagdef fdef; - if ((retval = copy_from_user((void *) &fdef, data, - sizeof(kcapi_flagdef)))) - return retval; + if (copy_from_user((void *)&fdef, data, sizeof(kcapi_flagdef))) + return -EFAULT; card = get_capi_ctr_by_nr(fdef.contr); if (!card) diff -Nru a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c --- a/drivers/isdn/divert/divert_procfs.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/divert/divert_procfs.c Sun May 19 03:59:20 2002 @@ -185,8 +185,8 @@ divert_rule *rulep; char *cp; - if ((i = copy_from_user(&dioctl, (char *) arg, sizeof(dioctl)))) - return (i); + if (copy_from_user(&dioctl, (char *) arg, sizeof(dioctl))) + return -EFAULT; switch (cmd) { case IIOCGETVER: @@ -254,7 +254,7 @@ default: return (-EINVAL); } /* switch cmd */ - return (copy_to_user((char *) arg, &dioctl, sizeof(dioctl))); /* success */ + return copy_to_user((char *)arg, &dioctl, sizeof(dioctl)) ? -EFAULT : 0; } /* isdn_divert_ioctl */ diff -Nru a/drivers/isdn/eicon/eicon_mod.c b/drivers/isdn/eicon/eicon_mod.c --- a/drivers/isdn/eicon/eicon_mod.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/eicon/eicon_mod.c Sun May 19 03:59:20 2002 @@ -213,7 +213,10 @@ return(EICON_CTRL_VERSION); case EICON_IOCTL_GETTYPE: if (card->bus == EICON_BUS_PCI) { - copy_to_user((char *)a, &card->hwif.pci.master, sizeof(int)); + if (copy_to_user((char *)a, + &card->hwif.pci.master, + sizeof(int))) + return -EFAULT; } return(card->type); case EICON_IOCTL_GETMMIO: @@ -351,7 +354,8 @@ return -ENODEV; case EICON_IOCTL_ADDCARD: - if ((ret = copy_from_user(&cdef, (char *)a, sizeof(cdef)))) + if (copy_from_user(&cdef, (char *)a, + sizeof(cdef))) return -EFAULT; if (!(eicon_addcard(0, cdef.membase, cdef.irq, cdef.id, 0))) return -EIO; @@ -376,8 +380,9 @@ #ifdef CONFIG_ISDN_DRV_EICON_PCI if (c->arg < EICON_IOCTL_DIA_OFFSET) return -EINVAL; - if (copy_from_user(&dstart, (char *)a, sizeof(dstart))) - return -1; + if (copy_from_user(&dstart, (char *)a, + sizeof(dstart))) + return -EFAULT; if (!(card = eicon_findnpcicard(dstart.card_id))) return -EINVAL; ret = do_ioctl(NULL, NULL, @@ -667,7 +672,8 @@ if (user) { spin_unlock_irqrestore(&eicon_lock, flags); - copy_to_user(p, skb->data, cnt); + if (copy_to_user(p, skb->data, cnt)) + return -EFAULT; spin_lock_irqsave(&eicon_lock, flags); } else diff -Nru a/drivers/isdn/hardware/avm/b1.c b/drivers/isdn/hardware/avm/b1.c --- a/drivers/isdn/hardware/avm/b1.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/hardware/avm/b1.c Sun May 19 03:59:20 2002 @@ -166,15 +166,14 @@ { unsigned char buf[256]; unsigned char *dp; - int i, left, retval; + int i, left; unsigned int base = card->port; dp = t4file->data; left = t4file->len; while (left > sizeof(buf)) { if (t4file->user) { - retval = copy_from_user(buf, dp, sizeof(buf)); - if (retval) + if (copy_from_user(buf, dp, sizeof(buf))) return -EFAULT; } else { memcpy(buf, dp, sizeof(buf)); @@ -190,8 +189,7 @@ } if (left) { if (t4file->user) { - retval = copy_from_user(buf, dp, left); - if (retval) + if (copy_from_user(buf, dp, left)) return -EFAULT; } else { memcpy(buf, dp, left); @@ -211,7 +209,7 @@ unsigned char buf[256]; unsigned char *dp; unsigned int base = card->port; - int i, j, left, retval; + int i, j, left; dp = config->data; left = config->len; @@ -223,8 +221,7 @@ } while (left > sizeof(buf)) { if (config->user) { - retval = copy_from_user(buf, dp, sizeof(buf)); - if (retval) + if (copy_from_user(buf, dp, sizeof(buf))) return -EFAULT; } else { memcpy(buf, dp, sizeof(buf)); @@ -240,8 +237,7 @@ } if (left) { if (config->user) { - retval = copy_from_user(buf, dp, left); - if (retval) + if (copy_from_user(buf, dp, left)) return -EFAULT; } else { memcpy(buf, dp, left); diff -Nru a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c --- a/drivers/isdn/hardware/avm/c4.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/hardware/avm/c4.c Sun May 19 03:59:20 2002 @@ -191,15 +191,14 @@ { u32 val; unsigned char *dp; - int left, retval; + int left; u32 loadoff = 0; dp = t4file->data; left = t4file->len; while (left >= sizeof(u32)) { if (t4file->user) { - retval = copy_from_user(&val, dp, sizeof(val)); - if (retval) + if (copy_from_user(&val, dp, sizeof(val))) return -EFAULT; } else { memcpy(&val, dp, sizeof(val)); @@ -216,8 +215,7 @@ if (left) { val = 0; if (t4file->user) { - retval = copy_from_user(&val, dp, left); - if (retval) + if (copy_from_user(&val, dp, left)) return -EFAULT; } else { memcpy(&val, dp, left); @@ -808,8 +806,7 @@ left = config->len; while (left >= sizeof(u32)) { if (config->user) { - retval = copy_from_user(val, dp, sizeof(val)); - if (retval) + if (copy_from_user(val, dp, sizeof(val))) return -EFAULT; } else { memcpy(val, dp, sizeof(val)); @@ -822,8 +819,7 @@ if (left) { memset(val, 0, sizeof(val)); if (config->user) { - retval = copy_from_user(&val, dp, left); - if (retval) + if (copy_from_user(&val, dp, left)) return -EFAULT; } else { memcpy(&val, dp, left); diff -Nru a/drivers/isdn/hisax/config.c b/drivers/isdn/hisax/config.c --- a/drivers/isdn/hisax/config.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/hisax/config.c Sun May 19 03:59:20 2002 @@ -641,9 +641,10 @@ count = cs->status_end - cs->status_read + 1; if (count >= len) count = len; - if (user) - copy_to_user(p, cs->status_read, count); - else + if (user) { + if (copy_to_user(p, cs->status_read, count)) + return -EFAULT; + } else memcpy(p, cs->status_read, count); cs->status_read += count; if (cs->status_read > cs->status_end) @@ -655,9 +656,10 @@ cnt = HISAX_STATUS_BUFSIZE; else cnt = count; - if (user) - copy_to_user(p, cs->status_read, cnt); - else + if (user) { + if (copy_to_user(p, cs->status_read, cnt)) + return -EFAULT; + } else memcpy(p, cs->status_read, cnt); p += cnt; cs->status_read += cnt % HISAX_STATUS_BUFSIZE; diff -Nru a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c --- a/drivers/isdn/hisax/isar.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/hisax/isar.c Sun May 19 03:59:20 2002 @@ -217,7 +217,7 @@ } if ((ret = copy_from_user(&size, p, sizeof(int)))) { printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret); - return ret; + return -EFAULT; } p += sizeof(int); printk(KERN_DEBUG"isar_load_firmware size: %d\n", size); @@ -240,6 +240,7 @@ while (cnt < size) { if ((ret = copy_from_user(&blk_head, p, BLK_HEAD_SIZE))) { printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret); + ret = -EFAULT; goto reterror; } #ifdef __BIG_ENDIAN @@ -282,6 +283,7 @@ *mp++ = noc; if ((ret = copy_from_user(tmpmsg, p, nom))) { printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret); + ret = -EFAULT; goto reterror; } p += nom; diff -Nru a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c --- a/drivers/isdn/i4l/isdn_ppp.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/i4l/isdn_ppp.c Sun May 19 03:59:20 2002 @@ -731,7 +731,11 @@ restore_flags(flags); - copy_to_user(buf, save_buf, count); + if (copy_to_user(buf, save_buf, count)) { + kfree(save_buf); + retval = -EFAULT; + goto out; + } kfree(save_buf); retval = count; diff -Nru a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c --- a/drivers/isdn/i4l/isdn_tty.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/i4l/isdn_tty.c Sun May 19 03:59:20 2002 @@ -1202,9 +1202,12 @@ &(m->pluscount), &(m->lastplus), from_user); - if (from_user) - copy_from_user(&(info->xmit_buf[info->xmit_count]), buf, c); - else + if (from_user) { + if (copy_from_user(&(info->xmit_buf[info->xmit_count]), buf, c)) { + total = -EFAULT; + goto out; + } + } else memcpy(&(info->xmit_buf[info->xmit_count]), buf, c); #ifdef CONFIG_ISDN_AUDIO if (info->vonline) { @@ -1284,6 +1287,7 @@ } isdn_timer_ctrl(ISDN_TIMER_MODEMXMIT, 1); } +out: if (from_user) up(&info->write_sem); return total; @@ -2589,7 +2593,8 @@ *pluscount = 0; } if (from_user) { - copy_from_user(cbuf, p, count); + if (copy_from_user(cbuf, p, count)) + return; p = cbuf; } while (count > 0) { diff -Nru a/drivers/isdn/icn/icn.c b/drivers/isdn/icn/icn.c --- a/drivers/isdn/icn/icn.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/icn/icn.c Sun May 19 03:59:20 2002 @@ -821,9 +821,9 @@ printk(KERN_WARNING "icn: Could not allocate code buffer\n"); return -ENOMEM; } - if ((ret = copy_from_user(codebuf, buffer, ICN_CODE_STAGE1))) { + if (copy_from_user(codebuf, buffer, ICN_CODE_STAGE1)) { kfree(codebuf); - return ret; + return -EFAULT; } if (!card->rvalid) { if (check_region(card->port, ICN_PORTLEN)) { @@ -1057,9 +1057,10 @@ count = cmd_free; if (count > len) count = len; - if (user) - copy_from_user(msg, buf, count); - else + if (user) { + if (copy_from_user(msg, buf, count)) + return -EFAULT; + } else memcpy(msg, buf, count); save_flags(flags); @@ -1237,15 +1238,17 @@ case ICN_IOCTL_GETDOUBLE: return (int) card->doubleS0; case ICN_IOCTL_DEBUGVAR: - if ((i = copy_to_user((char *) a, - (char *) &card, sizeof(ulong)))) - return i; + if (copy_to_user((char *)a, + (char *)&card, + sizeof(ulong))) + return -EFAULT; a += sizeof(ulong); { ulong l = (ulong) & dev; - if ((i = copy_to_user((char *) a, - (char *) &l, sizeof(ulong)))) - return i; + if (copy_to_user((char *)a, + (char *)&l, + sizeof(ulong))) + return -EFAULT; } return 0; case ICN_IOCTL_LOADBOOT: @@ -1266,8 +1269,10 @@ case ICN_IOCTL_ADDCARD: if (!dev.firstload) return -EBUSY; - if ((i = copy_from_user((char *) &cdef, (char *) a, sizeof(cdef)))) - return i; + if (copy_from_user((char *)&cdef, + (char *)a, + sizeof(cdef))) + return -EFAULT; return (icn_addcard(cdef.port, cdef.id1, cdef.id2)); break; case ICN_IOCTL_LEASEDCFG: diff -Nru a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c --- a/drivers/isdn/isdnloop/isdnloop.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/isdnloop/isdnloop.c Sun May 19 03:59:20 2002 @@ -986,9 +986,10 @@ if (count > 255) count = 255; - if (user) - copy_from_user(msg, buf, count); - else + if (user) { + if (copy_from_user(msg, buf, count)) + return -EFAULT; + } else memcpy(msg, buf, count); isdnloop_putmsg(card, '>'); for (p = msg; count > 0; count--, p++) { @@ -1076,7 +1077,8 @@ if (card->flags & ISDNLOOP_FLAGS_RUNNING) return -EBUSY; - copy_from_user((char *) &sdef, (char *) sdefp, sizeof(sdef)); + if (copy_from_user((char *) &sdef, (char *) sdefp, sizeof(sdef))) + return -EFAULT; save_flags(flags); cli(); switch (sdef.ptype) { @@ -1149,9 +1151,10 @@ return (isdnloop_start(card, (isdnloop_sdef *) a)); break; case ISDNLOOP_IOCTL_ADDCARD: - if ((i = verify_area(VERIFY_READ, (void *) a, sizeof(isdnloop_cdef)))) - return i; - copy_from_user((char *) &cdef, (char *) a, sizeof(cdef)); + if (copy_from_user((char *)&cdef, + (char *)a, + sizeof(cdef))) + return -EFAULT; return (isdnloop_addcard(cdef.id1)); break; case ISDNLOOP_IOCTL_LEASEDCFG: diff -Nru a/drivers/isdn/sc/command.c b/drivers/isdn/sc/command.c --- a/drivers/isdn/sc/command.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/sc/command.c Sun May 19 03:59:20 2002 @@ -126,11 +126,11 @@ int err; memcpy(&cmdptr, cmd->parm.num, sizeof(unsigned long)); - if((err = copy_from_user(&ioc, (scs_ioctl *) cmdptr, - sizeof(scs_ioctl)))) { + if (copy_from_user(&ioc, (scs_ioctl *)cmdptr, + sizeof(scs_ioctl))) { pr_debug("%s: Failed to verify user space 0x%x\n", adapter[card]->devicename, cmdptr); - return err; + return -EFAULT; } return sc_ioctl(card, &ioc); } diff -Nru a/drivers/isdn/sc/ioctl.c b/drivers/isdn/sc/ioctl.c --- a/drivers/isdn/sc/ioctl.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/sc/ioctl.c Sun May 19 03:59:20 2002 @@ -55,8 +55,8 @@ /* * Get the SRec from user space */ - if ((err = copy_from_user(srec, (char *) data->dataptr, sizeof(srec)))) - return err; + if (copy_from_user(srec, (char *) data->dataptr, sizeof(srec))) + return -EFAULT; status = send_and_receive(card, CMPID, cmReqType2, cmReqClass0, cmReqLoadProc, 0, sizeof(srec), srec, &rcvmsg, SAR_TIMEOUT); @@ -96,8 +96,9 @@ /* * Get the switch type from user space */ - if ((err = copy_from_user(&switchtype, (char *) data->dataptr, sizeof(char)))) - return err; + if (copy_from_user(&switchtype, (char *)data->dataptr, + sizeof(char))) + return -EFAULT; pr_debug("%s: SCIOCSETSWITCH: setting switch type to %d\n", adapter[card]->devicename, switchtype); @@ -141,8 +142,9 @@ /* * Package the switch type and send to user space */ - if ((err = copy_to_user((char *) data->dataptr, &switchtype, sizeof(char)))) - return err; + if (copy_to_user((char *)data->dataptr, &switchtype, + sizeof(char))) + return -EFAULT; return 0; } @@ -173,8 +175,8 @@ /* * Package the switch type and send to user space */ - if ((err = copy_to_user((char *) data->dataptr, spid, sizeof(spid)))) - return err; + if (copy_to_user((char *)data->dataptr, spid, sizeof(spid))) + return -EFAULT; return 0; } @@ -190,8 +192,8 @@ /* * Get the spid from user space */ - if ((err = copy_from_user(spid, (char *) data->dataptr, sizeof(spid)))) - return err; + if (copy_from_user(spid, (char *) data->dataptr, sizeof(spid))) + return -EFAULT; pr_debug("%s: SCIOCSETSPID: setting channel %d spid to %s\n", adapter[card]->devicename, data->channel, spid); @@ -237,8 +239,8 @@ /* * Package the dn and send to user space */ - if ((err = copy_to_user((char *) data->dataptr, dn, sizeof(dn)))) - return err; + if (copy_to_user((char *)data->dataptr, dn, sizeof(dn))) + return -EFAULT; return 0; } @@ -254,8 +256,8 @@ /* * Get the spid from user space */ - if ((err = copy_from_user(dn, (char *) data->dataptr, sizeof(dn)))) - return err; + if (copy_from_user(dn, (char *)data->dataptr, sizeof(dn))) + return -EFAULT; pr_debug("%s: SCIOCSETDN: setting channel %d dn to %s\n", adapter[card]->devicename, data->channel, dn); @@ -290,8 +292,9 @@ pr_debug("%s: SCIOSTAT: ioctl received\n", adapter[card]->devicename); GetStatus(card, &bi); - if ((err = copy_to_user((boardInfo *) data->dataptr, &bi, sizeof(boardInfo)))) - return err; + if (copy_to_user((boardInfo *)data->dataptr, &bi, + sizeof(boardInfo))) + return -EFAULT; return 0; } @@ -324,8 +327,8 @@ /* * Package the switch type and send to user space */ - if ((err = copy_to_user((char *) data->dataptr, &speed, sizeof(char)))) - return err; + if (copy_to_user((char *) data->dataptr, &speed, sizeof(char))) + return -EFAULT; return 0; } diff -Nru a/drivers/isdn/tpam/tpam_commands.c b/drivers/isdn/tpam/tpam_commands.c --- a/drivers/isdn/tpam/tpam_commands.c Sun May 19 03:59:20 2002 +++ b/drivers/isdn/tpam/tpam_commands.c Sun May 19 03:59:20 2002 @@ -126,7 +126,6 @@ */ static int tpam_command_ioctl_dspload(tpam_card *card, u32 arg) { tpam_dsp_ioctl tdl; - int ret; dprintk("TurboPAM(tpam_command_ioctl_dspload): card=%d\n", card->id); @@ -141,10 +140,9 @@ return -EPERM; /* write the data in the board's memory */ - ret = copy_from_user_to_pam(card, (void *)tdl.address, - (void *)arg + sizeof(tpam_dsp_ioctl), - tdl.data_len); - return 0; + return copy_from_user_to_pam(card, (void *)tdl.address, + (void *)arg + sizeof(tpam_dsp_ioctl), + tdl.data_len); } /* @@ -158,7 +156,6 @@ */ static int tpam_command_ioctl_dspsave(tpam_card *card, u32 arg) { tpam_dsp_ioctl tdl; - int ret; dprintk("TurboPAM(tpam_command_ioctl_dspsave): card=%d\n", card->id); @@ -171,9 +168,8 @@ return -EPERM; /* read the data from the board's memory */ - ret = copy_from_pam_to_user(card, (void *)arg + sizeof(tpam_dsp_ioctl), - (void *)tdl.address, tdl.data_len); - return ret; + return copy_from_pam_to_user(card, (void *)arg + sizeof(tpam_dsp_ioctl), + (void *)tdl.address, tdl.data_len); } /* ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 4:18 AUDIT of 2.5.15 copy_to/from_user Rusty Russell 2002-05-18 22:55 ` Arnaldo Carvalho de Melo @ 2002-05-19 11:44 ` Alan Cox 2002-05-19 12:10 ` Stephen Rothwell ` (2 more replies) 2002-05-19 12:13 ` Alan Cox 2002-05-19 12:48 ` Arnaldo Carvalho de Melo 3 siblings, 3 replies; 30+ messages in thread From: Alan Cox @ 2002-05-19 11:44 UTC (permalink / raw) To: Rusty Russell; +Cc: linux-kernel, kernel-janitor-discuss Looking at 2.4.1x which has the same signal code > arch/i386/kernel/signal.c:37: return __copy_to_user(to, from, sizeof(siginfo_t)); not a bug > arch/sparc/kernel/signal.c:101: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/alpha/kernel/signal.c:44: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/mips/kernel/signal.c:45: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/ppc/kernel/signal.c:70: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/m68k/kernel/signal.c:198: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/sparc64/kernel/signal.c:49: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/arm/kernel/signal.c:62: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/sh/kernel/signal.c:42: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/ia64/kernel/signal.c:147: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/mips64/kernel/signal.c:45: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/s390/kernel/signal.c:57: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/cris/kernel/signal.c:51: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/cris/kernel/signal.c:51: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug > arch/x86_64/kernel/signal.c:47: return __copy_to_user(to, from, sizeof(siginfo_t)); Not a bug ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 11:44 ` Alan Cox @ 2002-05-19 12:10 ` Stephen Rothwell 2002-05-19 12:15 ` Rui Sousa 2002-05-20 1:38 ` Rusty Russell 2 siblings, 0 replies; 30+ messages in thread From: Stephen Rothwell @ 2002-05-19 12:10 UTC (permalink / raw) To: Alan Cox; +Cc: rusty, linux-kernel, kernel-janitor-discuss On Sun, 19 May 2002 12:44:30 +0100 (BST) Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > > > arch/ia64/kernel/signal.c:147: return __copy_to_user(to, from, sizeof(siginfo_t)); > > Not a bug Actually a bug as sys_ptrace on ia64 assumes that copy_siginfo_to_user returns 0 or -Esomething. -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 11:44 ` Alan Cox 2002-05-19 12:10 ` Stephen Rothwell @ 2002-05-19 12:15 ` Rui Sousa 2002-05-19 12:46 ` Alan Cox 2002-05-20 1:38 ` Rusty Russell 2 siblings, 1 reply; 30+ messages in thread From: Rui Sousa @ 2002-05-19 12:15 UTC (permalink / raw) To: Alan Cox; +Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss On Sun, 19 May 2002, Alan Cox wrote: > Looking at 2.4.1x which has the same signal code > > > arch/i386/kernel/signal.c:37: return __copy_to_user(to, from, sizeof(siginfo_t)); > > not a bug > > arch/sparc/kernel/signal.c:101: return __copy_to_user(to, from, sizeof(siginfo_t)); > > Not a bug > > > arch/alpha/kernel/signal.c:44: return __copy_to_user(to, from, sizeof(siginfo_t)); > > Not a bug > Hi, Halfway this thread I got I bit confused... copy_to/from_user() --> will return the number of bytes that were _not_ copied. If one does not care about partially successes just use: if (copy_to/from_user()) return -EFAULT; __copy_to/from_user() --> the same as above, but can they actually return anything other than 0? My assembler is no good and I'm not able to see if the macros __constant_copy_user(), __copy_user(), __constant_copy_user_zeroing(), modify the size argument. Rui Sousa ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 12:15 ` Rui Sousa @ 2002-05-19 12:46 ` Alan Cox 2002-05-19 12:58 ` Rui Sousa 0 siblings, 1 reply; 30+ messages in thread From: Alan Cox @ 2002-05-19 12:46 UTC (permalink / raw) To: Rui Sousa; +Cc: Alan Cox, Rusty Russell, linux-kernel, kernel-janitor-discuss > copy_to/from_user() --> will return the number of bytes that were _not_ > copied. If one does not care about partially successes just use: > > if (copy_to/from_user()) > return -EFAULT; Yes > __copy_to/from_user() --> the same as above, but can they actually return > anything other than 0? My assembler is no good and I'm not able to see if They do the same things, but don't do any initial range checks that might be done by access_ok before hand ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 12:46 ` Alan Cox @ 2002-05-19 12:58 ` Rui Sousa 2002-05-19 13:43 ` Alan Cox 0 siblings, 1 reply; 30+ messages in thread From: Rui Sousa @ 2002-05-19 12:58 UTC (permalink / raw) To: Alan Cox; +Cc: Rusty Russell, linux-kernel, kernel-janitor-discuss On Sun, 19 May 2002, Alan Cox wrote: > > copy_to/from_user() --> will return the number of bytes that were _not_ > > copied. If one does not care about partially successes just use: > > > > if (copy_to/from_user()) > > return -EFAULT; > > Yes > > > __copy_to/from_user() --> the same as above, but can they actually return > > anything other than 0? My assembler is no good and I'm not able to see if > > They do the same things, but don't do any initial range checks that might > be done by access_ok before hand On the emu10k1 driver we use access_ok(VERIFY_READ) once at the beginning of the write() routine to check we can access the user buffer. After that we always use __copy_from_user() and we trust it not to fail. Is this correct, or not? Basically, where in copy_from_user() is it determined the function cannot copy the entire user buffer? Is it in access_ok() only or also in __constant_copy_user_zeroing()? Rui ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 12:58 ` Rui Sousa @ 2002-05-19 13:43 ` Alan Cox 2002-05-19 17:01 ` Hugh Dickins 2002-05-19 17:52 ` David Woodhouse 0 siblings, 2 replies; 30+ messages in thread From: Alan Cox @ 2002-05-19 13:43 UTC (permalink / raw) To: Rui Sousa; +Cc: Alan Cox, Rusty Russell, linux-kernel, kernel-janitor-discuss > > > __copy_to/from_user() --> the same as above, but can they actually return > > > anything other than 0? My assembler is no good and I'm not able to see if > > > > They do the same things, but don't do any initial range checks that might > > be done by access_ok before hand > > On the emu10k1 driver we use access_ok(VERIFY_READ) once at the beginning > of the write() routine to check we can access the user buffer. After that > we always use __copy_from_user() and we trust it not to fail. Is this > correct, or not? This is correct > Basically, where in copy_from_user() is it determined the function cannot > copy the entire user buffer? Is it in access_ok() only or also in > __constant_copy_user_zeroing()? Static once off checks are done in access_ok Dynamic checks are doing in __copy_from_* Which are which depends on the platform. On x86 for example access_ok is basically a check for 0->0xBFFFFFFF range and no more ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 13:43 ` Alan Cox @ 2002-05-19 17:01 ` Hugh Dickins 2002-05-19 17:36 ` Alan Cox 2002-05-19 17:52 ` David Woodhouse 1 sibling, 1 reply; 30+ messages in thread From: Hugh Dickins @ 2002-05-19 17:01 UTC (permalink / raw) To: Alan Cox; +Cc: Rui Sousa, Rusty Russell, linux-kernel, kernel-janitor-discuss On Sun, 19 May 2002, Alan Cox wrote: > > > > On the emu10k1 driver we use access_ok(VERIFY_READ) once at the beginning > > of the write() routine to check we can access the user buffer. After that > > we always use __copy_from_user() and we trust it not to fail. Is this > > correct, or not? > > This is correct Am I right to read that as "This is a correct description of what is currently done in the emu10k1 driver, but what it is doing is incorrect"? > Static once off checks are done in access_ok > Dynamic checks are doing in __copy_from_* > > Which are which depends on the platform. On x86 for example access_ok > is basically a check for 0->0xBFFFFFFF range and no more Hugh ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 17:01 ` Hugh Dickins @ 2002-05-19 17:36 ` Alan Cox 0 siblings, 0 replies; 30+ messages in thread From: Alan Cox @ 2002-05-19 17:36 UTC (permalink / raw) To: Hugh Dickins Cc: Alan Cox, Rui Sousa, Rusty Russell, linux-kernel, kernel-janitor-discuss > On Sun, 19 May 2002, Alan Cox wrote: > > > > > > On the emu10k1 driver we use access_ok(VERIFY_READ) once at the beginning > > > of the write() routine to check we can access the user buffer. After that > > > we always use __copy_from_user() and we trust it not to fail. Is this > > > correct, or not? > > > > This is correct > > Am I right to read that as "This is a correct description of what is > currently done in the emu10k1 driver, but what it is doing is incorrect"? It seems correct in what it is doing, to a point. copy_from_user won't ever fail in a fatal manner. It may not copy enough data and zero fill returning an error code. Reporting of that error isnt required however, its just good manners ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 13:43 ` Alan Cox 2002-05-19 17:01 ` Hugh Dickins @ 2002-05-19 17:52 ` David Woodhouse 2002-05-19 18:20 ` Alan Cox 1 sibling, 1 reply; 30+ messages in thread From: David Woodhouse @ 2002-05-19 17:52 UTC (permalink / raw) To: Alan Cox; +Cc: Rui Sousa, Rusty Russell, linux-kernel, kernel-janitor-discuss alan@lxorguk.ukuu.org.uk said: > > On the emu10k1 driver we use access_ok(VERIFY_READ) once at the > > beginning of the write() routine to check we can access the user buffer. > > After that we always use __copy_from_user() and we trust it not to fail. > > Is this correct, or not? > This is correct Even if another thread unmaps the page we were trying to read from between the access_ok() and the actual copy? -- dwmw2 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 17:52 ` David Woodhouse @ 2002-05-19 18:20 ` Alan Cox 2002-05-19 18:02 ` David Woodhouse 0 siblings, 1 reply; 30+ messages in thread From: Alan Cox @ 2002-05-19 18:20 UTC (permalink / raw) To: David Woodhouse Cc: Alan Cox, Rui Sousa, Rusty Russell, linux-kernel, kernel-janitor-discuss > > > After that we always use __copy_from_user() and we trust it not to fail. > > > Is this correct, or not? > > > This is correct > > Even if another thread unmaps the page we were trying to read from between > the access_ok() and the actual copy? Yes. The only check done in access_ok on x86 is the 0xC0000000->0xFFFFFFFF check with isnt affected by remappings. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 18:20 ` Alan Cox @ 2002-05-19 18:02 ` David Woodhouse 2002-05-19 22:54 ` Alan Cox 0 siblings, 1 reply; 30+ messages in thread From: David Woodhouse @ 2002-05-19 18:02 UTC (permalink / raw) To: Alan Cox; +Cc: Rui Sousa, Rusty Russell, linux-kernel, kernel-janitor-discuss alan@lxorguk.ukuu.org.uk said: > > > After that we always use __copy_from_user() and we trust it not to fail. > > This is correct > The only check done in access_ok on x86 is the 0xC0000000->0xFFFFFFFF > check with isnt affected by remappings. Right.... so trusting __copy_to_user() not to fail doesn't seem particularly correct. -- dwmw2 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 18:02 ` David Woodhouse @ 2002-05-19 22:54 ` Alan Cox 0 siblings, 0 replies; 30+ messages in thread From: Alan Cox @ 2002-05-19 22:54 UTC (permalink / raw) To: David Woodhouse Cc: Alan Cox, Rui Sousa, Rusty Russell, linux-kernel, kernel-janitor-discuss > alan@lxorguk.ukuu.org.uk said: > > > > After that we always use __copy_from_user() and we trust it not to fail. > > > This is correct > > > The only check done in access_ok on x86 is the 0xC0000000->0xFFFFFFFF > > check with isnt affected by remappings. > > Right.... so trusting __copy_to_user() not to fail doesn't seem > particularly correct. EFAULT reporting is not required by posix. Its not so much incorrect as slightly rude.. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 11:44 ` Alan Cox 2002-05-19 12:10 ` Stephen Rothwell 2002-05-19 12:15 ` Rui Sousa @ 2002-05-20 1:38 ` Rusty Russell 2002-05-20 11:47 ` Alan Cox 2 siblings, 1 reply; 30+ messages in thread From: Rusty Russell @ 2002-05-20 1:38 UTC (permalink / raw) To: Alan Cox; +Cc: linux-kernel, kernel-janitor-discuss In message <E179P70-0003dg-00@the-village.bc.nu> you write: > Looking at 2.4.1x which has the same signal code > > > arch/i386/kernel/signal.c:37: return __copy_to_user(to, from, sizeof(siginfo_t)); > > not a bug Disagree. May not cause problems at the moment, but a function which does: if (!access_ok (VERIFY_WRITE, to, sizeof(siginfo_t))) return -EFAULT; if (from->si_code < 0) return __copy_to_user(to, from, sizeof(siginfo_t)); Is clearly wrong, Rusty. -- Anyone who quotes me in their sig is an idiot. -- Rusty Russell. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-20 1:38 ` Rusty Russell @ 2002-05-20 11:47 ` Alan Cox 0 siblings, 0 replies; 30+ messages in thread From: Alan Cox @ 2002-05-20 11:47 UTC (permalink / raw) To: Rusty Russell; +Cc: Alan Cox, linux-kernel, kernel-janitor-discuss > Disagree. May not cause problems at the moment, but a function which > does: > > if (!access_ok (VERIFY_WRITE, to, sizeof(siginfo_t))) > return -EFAULT; > if (from->si_code < 0) > return __copy_to_user(to, from, sizeof(siginfo_t)); > > Is clearly wrong, Its a function that returns non zero for error. Its clearly right. It just doesn't conform to Rusty's grand vision. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 4:18 AUDIT of 2.5.15 copy_to/from_user Rusty Russell 2002-05-18 22:55 ` Arnaldo Carvalho de Melo 2002-05-19 11:44 ` Alan Cox @ 2002-05-19 12:13 ` Alan Cox 2002-06-07 8:54 ` David S. Miller 2002-05-19 12:48 ` Arnaldo Carvalho de Melo 3 siblings, 1 reply; 30+ messages in thread From: Alan Cox @ 2002-05-19 12:13 UTC (permalink / raw) To: Rusty Russell; +Cc: linux-kernel, kernel-janitor-discuss > drivers/char/nwflash.c:158: ret = copy_to_user(buf, (void *)(FLASH_BASE + p), count); Real bug - Fixed in next 2.4 -ac tree > drivers/scsi/scsi_ioctl.c:383: return copy_to_user(arg, dev->host->pci_dev->slot_name, Real bug - fixed in next 2.4 -ac tree > drivers/sgi/char/sgiserial.c:1239: return copy_to_user(retinfo,&tmp,sizeof(*retinfo)); Real bug - fixed in next 2.4 -ac tree > drivers/usb/misc/auerswald.c:1556: ret = copy_to_user(devinfo.buf, cp->dev_desc, u); Real bug - fixed in next 2.4 -ac tree > arch/sparc/kernel/sys_sunos.c:481: ret = copy_to_user(&name->sname[0], &system_utsname.sysname[0], sizeof(name->sname) - 1); Very broken. > arch/sparc64/kernel/sys_sparc32.c:3675: return copy_to_user(res32, kres, sizeof(*res32)); Looks very borked in several other spots > arch/mips64/kernel/linux32.c:1537: err |= __copy_from_user (p->mtext, &up->mtext, second); Real bug - fixed in next 2.4 -ac tree > arch/s390/kernel/debug.c:458: if ((rc = copy_to_user(user_buf + count, This one is interesting - its not just a misunderstanding of the API also a return of the wrong variable > arch/s390/kernel/ptrace.c:119: retval=copy_from_user((void *)realuseraddr,(void *)copyaddr,len); Not a bug > arch/s390/kernel/ptrace.c:345: if((ret=copy_from_user(&parea,(void *)addr,sizeof(parea)))==0) Real bug - fixed in next 2.4 -ac tree > arch/s390x/kernel/debug.c:458: if ((rc = copy_to_user(user_buf + count, As per s390 > arch/s390x/kernel/ptrace.c:119: retval = copy_from_user(realuserptr, copyptr, len); Not a bug > arch/s390x/kernel/ptrace.c:360: if((ret=copy_from_user(&parea,(void *)addr,sizeof(parea)))==0) Real bug - fixed in the next 2.4 -ac tree Alan ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 12:13 ` Alan Cox @ 2002-06-07 8:54 ` David S. Miller 0 siblings, 0 replies; 30+ messages in thread From: David S. Miller @ 2002-06-07 8:54 UTC (permalink / raw) To: alan; +Cc: rusty, linux-kernel, kernel-janitor-discuss From: Alan Cox <alan@lxorguk.ukuu.org.uk> Date: Sun, 19 May 2002 13:13:41 +0100 (BST) > arch/sparc/kernel/sys_sunos.c:481: ret = copy_to_user(&name->sname[0], &system_utsname.sysname[0], sizeof(name->sname) - 1); Very broken. > arch/sparc64/kernel/sys_sparc32.c:3675: return copy_to_user(res32, kres, sizeof(*res32)); Looks very borked in several other spots I've taken care of these bits in my tree and double checked each file in it's entirety. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 4:18 AUDIT of 2.5.15 copy_to/from_user Rusty Russell ` (2 preceding siblings ...) 2002-05-19 12:13 ` Alan Cox @ 2002-05-19 12:48 ` Arnaldo Carvalho de Melo 2002-05-19 17:28 ` Arnaldo Carvalho de Melo 3 siblings, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 12:48 UTC (permalink / raw) To: Rusty Russell; +Cc: linux-kernel, kernel-janitor-discuss Em Sun, May 19, 2002 at 02:18:53PM +1000, Rusty Russell escreveu: > The following uses seem to be incorrect: copy_from_user and > copy_to_user return the number of bytes NOT copied on failure, not > -EFAULT. > > You can CC: fixups to trivial at rustcorp.com.au. > > (I didn't look for cases where the Torvalds/McVoy philosophy says we > should be returning a partial result on EFAULT: that's more complex). > > Thanks, > Rusty. > ================ > Some cases are endemic: whole subsystems or drivers where the author > obviously thought copy_from_user follows the kernel conventions: > sound/pci/*.c Heads up: I'm fixing now the sound/pci ones... - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: AUDIT of 2.5.15 copy_to/from_user 2002-05-19 12:48 ` Arnaldo Carvalho de Melo @ 2002-05-19 17:28 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2002-05-19 17:28 UTC (permalink / raw) To: Rusty Russell, linux-kernel, kernel-janitor-discuss Em Sun, May 19, 2002 at 09:48:18AM -0300, Arnaldo C. Melo escreveu: > Em Sun, May 19, 2002 at 02:18:53PM +1000, Rusty Russell escreveu: > > Some cases are endemic: whole subsystems or drivers where the author > > obviously thought copy_from_user follows the kernel conventions: Heads up: I'm fixing now the drivers/char ones... - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2002-06-07 8:58 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2002-05-19 4:18 AUDIT of 2.5.15 copy_to/from_user Rusty Russell 2002-05-18 22:55 ` Arnaldo Carvalho de Melo 2002-05-18 23:12 ` [BKPATCH] " Arnaldo Carvalho de Melo 2002-05-18 23:54 ` Arnaldo Carvalho de Melo 2002-05-19 0:14 ` [BKPATCH] OSS: " Arnaldo Carvalho de Melo 2002-05-19 0:19 ` Arnaldo Carvalho de Melo 2002-05-19 1:16 ` Arnaldo Carvalho de Melo 2002-05-19 1:38 ` [BKPATCH] USB: " Arnaldo Carvalho de Melo 2002-05-20 6:07 ` Greg KH 2002-05-19 6:30 ` Kai Germaschewski 2002-05-19 0:45 ` Arnaldo Carvalho de Melo 2002-05-19 1:07 ` [BKPATCH] ISDN: " Arnaldo Carvalho de Melo 2002-05-19 11:44 ` Alan Cox 2002-05-19 12:10 ` Stephen Rothwell 2002-05-19 12:15 ` Rui Sousa 2002-05-19 12:46 ` Alan Cox 2002-05-19 12:58 ` Rui Sousa 2002-05-19 13:43 ` Alan Cox 2002-05-19 17:01 ` Hugh Dickins 2002-05-19 17:36 ` Alan Cox 2002-05-19 17:52 ` David Woodhouse 2002-05-19 18:20 ` Alan Cox 2002-05-19 18:02 ` David Woodhouse 2002-05-19 22:54 ` Alan Cox 2002-05-20 1:38 ` Rusty Russell 2002-05-20 11:47 ` Alan Cox 2002-05-19 12:13 ` Alan Cox 2002-06-07 8:54 ` David S. Miller 2002-05-19 12:48 ` Arnaldo Carvalho de Melo 2002-05-19 17:28 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.