linux-kernel - Re: [PATCH] sound: don't use the deprecated function check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 08 Aug 2011 11:43:32 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Wang Shaoyan <stufever@...il.com>
Cc:	linux-kernel@...r.kernel.org, perex@...ex.cz,
	Wang Shaoyan <wangshaoyan.pt@...bao.com>
Subject: Re: [PATCH] sound: don't use the deprecated function check_region

At Mon, 8 Aug 2011 17:33:40 +0800,
Wang Shaoyan wrote:
> 
> 2011/8/8 Takashi Iwai <tiwai@...e.de>:
> > At Mon,  8 Aug 2011 16:07:12 +0800,
> > stufever@...il.com wrote:
> >>
> >> From: Wang Shaoyan <wangshaoyan.pt@...bao.com>
> >>
> >>   sound/oss/pss.c: In function 'configure_nonsound_components':
> >>   sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
> >>
> >> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@...bao.com>
> >> ---
> >>  sound/oss/pss.c |    3 ++-
> >>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/sound/oss/pss.c b/sound/oss/pss.c
> >> index 9b800ce..0fa26bd 100644
> >> --- a/sound/oss/pss.c
> >> +++ b/sound/oss/pss.c
> >> @@ -673,9 +673,10 @@ static void configure_nonsound_components(void)
> >>
> >>       if (pss_cdrom_port == -1) {     /* If cdrom port enablation wasn't requested */
> >>               printk(KERN_INFO "PSS: CDROM port not enabled.\n");
> >> -     } else if (check_region(pss_cdrom_port, 2)) {
> >> +     } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
> >>               printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
> >>       } else {
> >> +             release_region(pss_cdrom_port, 2);
> >>               set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> >>               printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
> >>       }
> >
> > Well, this just hides the warning but doesn't fix the possible race,
> > which is the reason check_region() is marked as deprecated after all.
> >
> 
> AFAIK, check_region() is deprecated because such usage:
> check_region()
> ...do something    <-- here another process may do check and request same region
> request_region()
> But nowadays, request_region has done the check internally, so use the
> proper request_region to check is enough, am I miss something?

Right, if it uses request_region() and keeps the resource until the
end of the driver life, it's fine.

> > Instead of releasing the resource there, keep it and release in the
> > destructor properly like other normal resources.
> >
> 
> maybe we can release here?
> 
> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
> release_region(pss_cdrom_port, 2);

No.  What happens if another driver takes this resource and starts
accessing?  It conflicts.  The driver must keep the resource by itself
to protect from others as long as it uses.


thanks,

Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/