linux-kernel - Re: [PATCH] xen/blkback: fix reference counting

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

Message-ID: <20130906133708.GM2590@phenom.dumpdata.com>
Date:	Fri, 6 Sep 2013 09:37:08 -0400
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	Vegard Nossum <vegard.nossum@...cle.com>
Cc:	Ian Campbell <Ian.Campbell@...rix.com>,
	linux-kernel@...r.kernel.org, xen-devel@...ts.xenproject.org,
	stable@...r.kernel.org, Jan Beulich <JBeulich@...e.com>
Subject: Re: [PATCH] xen/blkback: fix reference counting

On Thu, Sep 05, 2013 at 01:00:14PM +0200, Vegard Nossum wrote:
> If the permission check fails, we drop a reference to the blkif without
> having taken it in the first place. The bug was introduced in commit
> 604c499cbbcc3d5fe5fb8d53306aa0fae1990109 (xen/blkback: Check device
> permissions before allowing OP_DISCARD).
> 
> Cc: stable@...r.kernel.org
> Cc: Jan Beulich <JBeulich@...e.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>

Thank you for spotting that. Let me stick it on my for-jens queue.

> Signed-off-by: Vegard Nossum <vegard.nossum@...cle.com>
> ---
>  drivers/block/xen-blkback/blkback.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index bf4b9d2..6620b73 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -872,49 +872,50 @@ static int xen_blkbk_parse_indirect(struct blkif_request *req,
>  	}
>  
>  unmap:
>  	if (segments)
>  		kunmap_atomic(segments);
>  	xen_blkbk_unmap(blkif, pages, indirect_grefs);
>  	return rc;
>  }
>  
>  static int dispatch_discard_io(struct xen_blkif *blkif,
>  				struct blkif_request *req)
>  {
>  	int err = 0;
>  	int status = BLKIF_RSP_OKAY;
>  	struct block_device *bdev = blkif->vbd.bdev;
>  	unsigned long secure;
>  	struct phys_req preq;
>  
> +	xen_blkif_get(blkif);
> +
>  	preq.sector_number = req->u.discard.sector_number;
>  	preq.nr_sects      = req->u.discard.nr_sectors;
>  
>  	err = xen_vbd_translate(&preq, blkif, WRITE);
>  	if (err) {
>  		pr_warn(DRV_PFX "access denied: DISCARD [%llu->%llu] on dev=%04x\n",
>  			preq.sector_number,
>  			preq.sector_number + preq.nr_sects, blkif->vbd.pdevice);
>  		goto fail_response;
>  	}
>  	blkif->st_ds_req++;
>  
> -	xen_blkif_get(blkif);
>  	secure = (blkif->vbd.discard_secure &&
>  		 (req->u.discard.flag & BLKIF_DISCARD_SECURE)) ?
>  		 BLKDEV_DISCARD_SECURE : 0;
>  
>  	err = blkdev_issue_discard(bdev, req->u.discard.sector_number,
>  				   req->u.discard.nr_sectors,
>  				   GFP_KERNEL, secure);
>  fail_response:
>  	if (err == -EOPNOTSUPP) {
>  		pr_debug(DRV_PFX "discard op failed, not supported\n");
>  		status = BLKIF_RSP_EOPNOTSUPP;
>  	} else if (err)
>  		status = BLKIF_RSP_ERROR;
>  
>  	make_response(blkif, req->u.discard.id, req->operation, status);
>  	xen_blkif_put(blkif);
>  	return err;
>  }
> -- 
> 1.7.10.4
> 
--
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/