linux-kernel - Re: [PATCH] xen-blkfront: remove type check from blkfront_setup

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: <52D036FC.6000308@oracle.com>
Date:	Fri, 10 Jan 2014 13:07:56 -0500
From:	Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:	Olaf Hering <olaf@...fle.de>
CC:	konrad.wilk@...cle.com, xen-devel@...ts.xen.org,
	linux-kernel@...r.kernel.org, david.vrabel@...rix.com
Subject: Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard

On 01/10/2014 11:28 AM, Olaf Hering wrote:
> In its initial implementation a check for "type" was added, but only phy
> and file are handled. This breaks advertised discard support for other
> type values such as qdisk.
>
> Fix and simplify this function: If the backend advertises discard
> support it is supposed to implement it properly, so enable
> feature_discard unconditionally. If the backend advertises the need for
> a certain granularity and alignment then propagate both properties to
> the blocklayer. The discard-secure property is a boolean, update the code
> to reflect that.
>
> Signed-off-by: Olaf Hering <olaf@...fle.de>
> ---
>   drivers/block/xen-blkfront.c | 40 ++++++++++++++--------------------------
>   1 file changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index c4a4c90..c9e96b9 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -1635,36 +1635,24 @@ blkfront_closing(struct blkfront_info *info)
>   static void blkfront_setup_discard(struct blkfront_info *info)
>   {
>   	int err;
> -	char *type;
>   	unsigned int discard_granularity;
>   	unsigned int discard_alignment;
>   	unsigned int discard_secure;
>   
> -	type = xenbus_read(XBT_NIL, info->xbdev->otherend, "type", NULL);
> -	if (IS_ERR(type))
> -		return;
> -
> -	info->feature_secdiscard = 0;
> -	if (strncmp(type, "phy", 3) == 0) {
> -		err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> -			"discard-granularity", "%u", &discard_granularity,
> -			"discard-alignment", "%u", &discard_alignment,
> -			NULL);
> -		if (!err) {
> -			info->feature_discard = 1;
> -			info->discard_granularity = discard_granularity;
> -			info->discard_alignment = discard_alignment;
> -		}
> -		err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> -			    "discard-secure", "%d", &discard_secure,
> -			    NULL);
> -		if (!err)
> -			info->feature_secdiscard = discard_secure;
> -
> -	} else if (strncmp(type, "file", 4) == 0)
> -		info->feature_discard = 1;
> -
> -	kfree(type);
> +	info->feature_discard = 1;

If the call below fails, is it safe to continue using discard feature? 
At the least, are discard_granularity and discard_alignment guaranteed 
to have sane/safe values?

> +	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> +		"discard-granularity", "%u", &discard_granularity,
> +		"discard-alignment", "%u", &discard_alignment,
> +		NULL);
> +	if (!err) {
> +		info->discard_granularity = discard_granularity;
> +		info->discard_alignment = discard_alignment;
> +	}
> +	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> +		    "discard-secure", "%d", &discard_secure,
> +		    NULL);
> +	if (!err)
> +		info->feature_secdiscard = !!discard_secure;
>   }

err variable is not really necessary so you can drop it.


-boris
>   
>   static int blkfront_setup_indirect(struct blkfront_info *info)

--
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/