public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: viro@parcelfarce.linux.theplanet.co.uk
To: Omkhar Arasaratnam <omkhar@rogers.com>
Cc: axeboe@suse.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers/cdrom/isp16.c check_region() fix
Date: Mon, 29 Dec 2003 20:13:28 +0000	[thread overview]
Message-ID: <20031229201328.GM4176@parcelfarce.linux.theplanet.co.uk> (raw)
In-Reply-To: <20031229194222.GA26019@omkhar.ibm.com>

On Mon, Dec 29, 2003 at 02:42:23PM -0500, Omkhar Arasaratnam wrote:
> check_region is depreciated in 2.6, this replaces it with request_region
> 
> As this is my first patch to the kernel, please let me know if I did anything wrong

It's pointless - the reason why check_region() is bad applies to your code.
The problem with check_region() is simple - it gives no protection against
somebody else grabbing the ports in question just as you've got "it's free"
from check_region().  The same problem exists with your replacement -
as soon as you've released the region it could've been grabbed by anybody.

Note that there's another problem - on rmmod we release the region we
hadn't grabbed.  Proper fix is
	a) replace check_region with request_region
	b) check that we don't have any IO on those ports before that
point (surprisingly many drivers are buggy that way - they do some IO
and then go "oops, looks like somebody held these ports after all;
oh, well, let's hope we hadn't screwed them too badly").  AFAICS isp16
is OK in that repect, though.
	c) make sure that we release that region on all failure exits
past that point, so that insmod failure would not leave it grabbed.

      parent reply	other threads:[~2003-12-29 20:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-12-29 19:42 [PATCH] drivers/cdrom/isp16.c check_region() fix Omkhar Arasaratnam
2003-12-29 20:04 ` Christoph Hellwig
2003-12-29 20:13 ` viro [this message]

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=20031229201328.GM4176@parcelfarce.linux.theplanet.co.uk \
    --to=viro@parcelfarce.linux.theplanet.co.uk \
    --cc=axeboe@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=omkhar@rogers.com \
    /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