From: linas@austin.ibm.com (Linas Vepstas)
To: Amos Waterland <apw@us.ibm.com>
Cc: Andrew Morton <akpm@osdl.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
rubini@vision.unipv.it, device@lanana.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Chardev checking of overlapping ranges is incorrect.
Date: Tue, 8 Aug 2006 16:33:31 -0500 [thread overview]
Message-ID: <20060808213331.GW10638@austin.ibm.com> (raw)
In-Reply-To: <20060808205258.GA6111@kvasir.watson.ibm.com>
On Tue, Aug 08, 2006 at 04:52:58PM -0400, Amos Waterland wrote:
> On Mon, Aug 07, 2006 at 11:47:53PM -0700, Andrew Morton wrote:
> > On Mon, 7 Aug 2006 17:55:55 -0500 linas@austin.ibm.com (Linas Vepstas) wrote:
> > > The current code in register_chrdev_region() attempts to check
> > > for overlapping regions of minor device numbers, but performs
> > > that check incorrectly. For example, if a device with minor
> > > numbers 128, 129, 130 is registered first, and a device with
> > > minor number 3,4,5 is registered later, then the later range
> > > is incorrectly identified as "overlapping" (since 130>3),
> > > when clearly this is the wrong conclusion.
>
> Hi Andrew. It does fix the original bug, but it introduces the bug that
> Linas pointed out. After looking at Linas' fix and finding an
> off-by-one error in his code,
Ooops
> Can you please back out my original patch and use this instead? I have
> run it through the test harness and I am much more confident in its
> correctness. Linas can you take a look at this and make sure you agree?
Actually, there's still a problem. An added device could still
overlap with a previously added device and not be detected.
We should keep the devices in order, and check that the region
fits in between the last and the next device. So, for example,
the following will make the latest patch accept an invalid region:
First, add maj=x, minor=64-127
Next, add maj=x, minor=0-63
Next, add maj=x, minor=32-63
When going to insert the third chardev, the for-loop will catch
the first elt in the chain, do the bounds-check, and add it
without complaint. I'll try a patch shortly.
--linas
next prev parent reply other threads:[~2006-08-08 21:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-08-07 22:55 [PATCH] Chardev checking of overlapping ranges is incorrect Linas Vepstas
2006-08-07 23:00 ` Linas Vepstas
2006-08-08 6:47 ` Andrew Morton
2006-08-08 20:52 ` Amos Waterland
2006-08-08 21:33 ` Linas Vepstas [this message]
2006-08-08 22:20 ` Amos Waterland
2006-08-09 1:15 ` Linas Vepstas
2006-08-09 17:06 ` Amos Waterland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060808213331.GW10638@austin.ibm.com \
--to=linas@austin.ibm.com \
--cc=akpm@osdl.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=apw@us.ibm.com \
--cc=device@lanana.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rubini@vision.unipv.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox