On Tue, 05 Jun 2007 13:05:18 +0200 Yoann Padioleau <[email protected]> wrote:
> Andrew Morton <[email protected]> writes:
>
> >>
> >> net/wan/lmc/lmc_main.c | 2 +-
> >> scsi/megaraid/megaraid_mm.c | 2 +-
> >> usb/serial/io_ti.c | 2 +-
> >> usb/serial/ti_usb_3410_5052.c | 2 +-
> >> usb/serial/whiteheat.c | 6 +++---
> >> 5 files changed, 7 insertions(+), 7 deletions(-)
> >
> > This patch covers three areas of maintainer responsibility, so poor old me
> > has to split it up and redo the changelogs. Oh well.
>
> Sorry.
>
> For modifications that crosscut the kernel it is not very practical to
> look each time for the maintainer.
That's OK - I can take care of that
> We plan to send a patch that
> replaces some calls to kmalloc/memset with kzalloc; do we have to split
> it for each maintainer ?
Per subsystem: yes, please. That maps pretty well onto per-subdirectory so
I'd suggest that each patch only affect the files in a single directory.
> >> diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
> >> index ae132c1..750b3ef 100644
> >> --- a/drivers/net/wan/lmc/lmc_main.c
> >> +++ b/drivers/net/wan/lmc/lmc_main.c
> >> @@ -483,7 +483,7 @@ #endif /* end ifdef _DBG_EVENTLOG */
> >> break;
> >> }
> >>
> >> - data = kmalloc(xc.len, GFP_KERNEL);
> >> + data = kmalloc(xc.len, GFP_ATOMIC);
> >> if(data == 0x0){
> >> printk(KERN_WARNING "%s: Failed to allocate memory for copy\n", dev->name);
> >> ret = -ENOMEM;
> >
> > A few lines later we do:
> >
> > if(copy_from_user(data, xc.data, xc.len))
> >
> > which also is illegal under spinlock.
>
> I have launched my tool to detect such situations and I get this
> (in a diff-like format).
>
> --- /home/pad/kernels/git/linux-2.6/arch/cris/arch-v10/drivers/gpio.c 2007-03-27 15:12:38.000000000 +0200
> +++ /tmp/output.c 2007-06-05 11:38:47.000000000 +0200
> @@ -776,7 +776,7 @@
> /* bits set in *arg is set to input,
> * *arg updated with current input pins.
> */
> - if (copy_from_user(&val, (unsigned long*)arg, sizeof(val)))
> {
> ret = -EFAULT;
> break;
> @@ -789,7 +789,7 @@
> /* bits set in *arg is set to output,
> * *arg updated with current output pins.
> */
> - if (copy_from_user(&val, (unsigned long*)arg, sizeof(val)))
> {
> ret = -EFAULT;
> break;
>
>
> --- /home/pad/kernels/git/linux-2.6/arch/mips/au1000/common/power.c 2007-03-27 15:12:41.000000000 +0200
> +++ /tmp/output.c 2007-06-05 11:38:57.000000000 +0200
> @@ -330,7 +330,7 @@
> spin_unlock_irqrestore(&pm_lock, flags);
> return -EFAULT;
> }
> - if (copy_from_user(buf, buffer, *len)) {
> spin_unlock_irqrestore(&pm_lock, flags);
> return -EFAULT;
> }
>
> and some cases in lmc_main.c
OK.
>
> >
> >
> > Frankly, I think that a better use of this tool would be to just report on
> > the errors, let humans fix them up.
>
> Ok. Do you have a preference on the format ? a <file>:<line> format ?
file-n-line is good. If it can quote the relevant test then that's better
- line numbers change often.
Please always work against the very latest Linus git tree,
> Is there a place that gathered all those implicit programming rules
> (that copy_from_user must not be called inside a spinlock, etc) so that
> I can translate them in a script for our tool.
Not that I can think of, sorry.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
