[PATCH 2.4] i2c-dev user/kernel bug and mem leak
greg at kroah.com
Fri Aug 15 23:13:30 CEST 2003
On Thu, Aug 14, 2003 at 07:01:34PM -0700, Robert T. Johnson wrote:
> On Thu, 2003-08-14 at 12:09, Greg KH wrote:
> > Hm, much like Linus's sparse does already? :)
> Yes, but cqual needs fewer annotations (see below).
> > His checker missed the 2.6 version of this, for some reason, I haven't
> > taken the time to figure out why.
> Currently, sparse silently ignores missing annotations. Since i2c-dev.c
> is not extensively annotated, it missed this bug.
i2c-dev.c is annotated in 2.6. Did I miss anything that needs to be
marked as such?
> Also, cqual is more flexible than sparse. For example, i2c-dev.c wants
> to use some i2c_msg structures to hold user bufs, and some to hold
> kernel bufs. cqual handles this automatically, but sparse cannot at
> all. To get i2c-dev.c to work with sparse, you'd need to declare two
> new types, "struct kernel_i2c_msg" and "struct user_i2c_msg", and change
> every instance of i2c_msg to be one or the other.
That's something that will be necessary anyway, if we want to get this
to ever work on a 64 bit processor running with a 32 bit userspace
(amd64, ppc64, sparc64, etc.)
> > How is cqual going to handle all of the tty drivers which can have a
> > pointer be either a userspace pointer, or a kernel pointer depending on
> > the value of another paramater in a function?
> I think all these functions should be changed to take two pointers, only
> one of which is allowed to be non-NULL. Then the flag can go away. I
> hope to submit a patch to this effect in the future. I think sparse
> can't check these either, unless you insert casts between user/kernel.
> But inserting casts loses the benefits of the automatic verification,
> since the casts could be wrong.
Hm, how about just fixing the tty core to always pass in kernel buffers?
That would fix the "problem" in one place :)
Anyway, that's a 2.7 change that has been on my list of things to do for
> Ok. Here's a patch against 2.6.0-test3. I didn't add the md
> substructure to i2c_msg, since it would require changing lots of files
> throughout the kernel. If you think that's an important change, I'll do
> it. Otherwise, the patch is the same idea as before.
> Oh, yeah. This patch also fixes the mem leak, and includes the
> single-copy_from_user optimization you guys talked about earlier, since
> those haven't been merged into mainline linux yet.
Hm, I had already applied your patch, so this one doesn't apply. Care
to re-do it against 2.6.0-test4 whenever that comes out?
More information about the lm-sensors