linux-kernel - Re: [PATCH] sg: atomize check and set sdp->exclude in sg

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130605132746.GA1690@logfs.org>
Date:	Wed, 5 Jun 2013 09:27:46 -0400
From:	Jörn Engel <joern@...fs.org>
To:	vaughan <vaughan.cao@...cle.com>
Cc:	dgilbert@...erlog.com, JBottomley@...allels.com,
	linux-scsi@...r.kernel.org, xitao.cao@...il.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sg: atomize check and set sdp->exclude in sg_open

On Wed, 5 June 2013 17:18:33 +0800, vaughan wrote:
> 
> Check and set sdp->exclude should be atomic when set in sg_open().

The patch is line-wrapped.  More importantly, it doesn't seem to do
what your description indicates it should do.  And lastly, does this
fix a bug, possibly even one you have a testcase for, or was it found
by code inspection?

> Signed-off-by: Vaughan Cao <vaughan.cao@...cle.com>
> ---
>  drivers/scsi/sg.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 25b5455..0ede08f 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -245,6 +245,21 @@ static int set_exclude(Sg_device *sdp, char val)
>      return val;
>  }
> 
> +/* Check if we can set exclude and then set, return 1 if success */
> +static int try_set_exclude(Sg_device *sdp)
> +{
> +    unsigned long flags;
> +
> +    spin_lock_irqsave(&sg_open_exclusive_lock, flags);
> +    if (sdp->exclude) {
> +        spin_unlock_irqrestore(&sg_open_exclusive_lock, flags);
> +        return 0;
> +    }
> +    sdp->exclude = 1;
> +    spin_unlock_irqrestore(&sg_open_exclusive_lock, flags);
> +    return 1;
> +}
> +
>  static int sfds_list_empty(Sg_device *sdp)
>  {
>      unsigned long flags;
> @@ -303,7 +318,7 @@ sg_open(struct inode *inode, struct file *filp)
>              goto error_out;
>          }
>          res = wait_event_interruptible(sdp->o_excl_wait,
> -                       ((!sfds_list_empty(sdp) || get_exclude(sdp))
> ? 0 : set_exclude(sdp, 1)));
> +            ((!sfds_list_empty(sdp) || get_exclude(sdp)) ? 0 :
> try_set_exclude(sdp)));
>          if (res) {
>              retval = res;    /* -ERESTARTSYS because signal hit process */
>              goto error_out;
> -- 
> 1.7.11.7
> 

Jörn

--
Fantasy is more important than knowledge. Knowledge is limited,
while fantasy embraces the whole world.
-- Albert Einstein
--
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/