Re: [PATCH] remove net driver ugliness that sparse complains about

viro_at_parcelfarce.linux.theplanet.co.uk
Date: 05/29/04

  • Next message: Prakash K. Cheemplavam: "Re: 2.6.7-rc1-mm1: libata flooding my log"
    Date:	Sat, 29 May 2004 21:42:30 +0100
    To: Linus Torvalds <torvalds@osdl.org>
    
    

    On Sat, May 29, 2004 at 11:23:56AM -0700, Linus Torvalds wrote:
    > Since the whole point of sparse is to have _static_ typechecking, such
    > code will never be sparse-clean, and either we have to ignore it, or we
    > should split up the use into two different kinds of structures (with the
    > same members apart from the address space) and explicitly convert between
    > the two. I'd obviously prefer that approach, but it might be a fair
    > amount of work (most of it should be really trivial, though, and I suspect
    > it would clarify pointer usage a lot to know when a "struct msghdr" points
    > to user space, and when it points to kernel space. Or whatever - maybe
    > that was a bad example).

    Right now there is only one serious false positive I know about.
            put_user(0, dirent->d_name)
    and its equivanlents in some places. That's __typeof__() handling bug.

    The rest is easy to spot - ## handling is broken in minimally tricky
    cases, [arg] is not recognized in asm arguments, some __attribute__()
    are not recognized and string constant length limits sometimes bite
    in asm bodies.

    The rest of patchset (~360Kb right now, and it will grow more) does include
    several splittings of structs, BTW. It removes pretty much all noise on
    my alpha / amd64 / x86 builds; the rest is real issues.

    Probably the worst annoyance is iovec - there is almost no intersection
    between the code that expects kernel pointers in it with code that expects
    userland ones (majority). I hadn't split that one, but that's worth
    considering.

    sync kiocb is a disaster waiting to happen.

    ->write() of tty_driver will take some research - we might want to try and
    keep copying from userland in generic code instead of just splitting the
    method, but that will require figuring out the locking issues.

    A bunch of set_fs() users in compat code is simply broken and should be
    using compat_alloc_user_space() instead. They end up with a mix of kernel
    and userland pointers, and set_fs(KERNEL_DS) is not enough to handle that.

    console code has some moderately minor annoyances; compared to the ugliness
    of the entire code in that area they are not too interesting.

    One thing I would _really_ hate to see is use of typecasts just to shut
    sparse up - the point is to find the potentially problematic places, not
    to hide them. We probably need a flag for sparse that would warn about
    explicit typecasts changing noderef and address_space inside the pointers;
    it obviously won't help the casts to unsigned long and back, but those
    are presumably used when people really mean it.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Prakash K. Cheemplavam: "Re: 2.6.7-rc1-mm1: libata flooding my log"

    Relevant Pages

    • Re: "Enhanced" MD code avaible for review
      ... be required to make this work correctly using a userland approach. ... > kernel, and having to try harder to crash the kernel. ... protected *sooner* if the system crashes. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • [PATCH] broken bitmap_parse for ncpus > 32
      ... rather than drag over the kernel versions. ... I tested this in userland with the kernel's versions of bitmap_shift_* ... void bitmap_shift_right(unsigned long *dst, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: From Eric Anholt:
      ... >> care to comment? ... userland and kernel, and nothing else. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: "Enhanced" MD code avaible for review
      ... The RAID transforms which perform this operation are kernel ... For a "do it in userland" solution, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: input/touchscreen (was Re: who sets boot_params[].screen_info.orig_video_isVGA?)
      ... looks like it is not really supported by the stock kernel... ... pointers to wild patches? ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)