linux-kernel - Re: [PATCH] mtd: Introduce CONFIG_MTD_RESERVE

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: <1430431357.2187.43.camel@x220>
Date:	Fri, 01 May 2015 00:02:37 +0200
From:	Paul Bolle <pebolle@...cali.nl>
To:	Ben Shelton <ben.shelton@...com>
Cc:	dwmw2@...radead.org, computersforpeace@...il.com,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Jeff Westfahl <jeff.westfahl@...com>
Subject: Re: [PATCH] mtd: Introduce CONFIG_MTD_RESERVE_END

While you're discussing more substantial questions with Brian, I found
some nits.

On Wed, 2015-04-29 at 14:28 -0500, Ben Shelton wrote:
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig


> +config MTD_RESERVE_END
> +	int "Reserved space at the end of an all remaining space partition"
> +	depends on MTD_CMDLINE_PARTS = "y"

(The quotes are unneeded.)

MTD_CMDLINE_PARTS is tristate. Why does it need to be built-in for this
symbol?

> +	default 0
> +	---help---
> +	  Specify an amount of reserved space at the end of the last MTD
> +	  partition when the size is specified with '-' to denote all
> +	  remaining space.
> +
> +	  This can be useful if, for example, the BBT is stored at the end
> +	  of the flash, and you don't want those blocks counted as part of
> +	  the last MTD partition. This is less heavyweight than reserving
> +	  the BBT blocks with a separate MTD partition. The BBT marks its
> +	  own blocks as bad blocks, which prevents an MTD driver such as
> +	  UBI from getting an accurate count of the actual bad blocks in
> +	  the MTD partition that contains the BBT.
> +
> +	  The value is specified in bytes. As an example, a typical BBT
> +	  reserves four erase blocks, and a typical erase block size is
> +	  128kB. To reserve that much space at the end of the flash, the
> +	  value for this config option would be 524288.
> +
> +	  If unsure, use the default value of zero.

> --- a/drivers/mtd/cmdlinepart.c
> +++ b/drivers/mtd/cmdlinepart.c
> @@ -340,7 +340,8 @@ static int parse_cmdline_partitions(struct mtd_info *master,
>  			offset = part->parts[i].offset;
>  
>  		if (part->parts[i].size == SIZE_REMAINING)
> -			part->parts[i].size = master->size - offset;
> +			part->parts[i].size = master->size - offset -
> +					      CONFIG_MTD_RESERVE_END;

I haven't tested this. (Quite often that means: I'm wrong.) But if
MTD_CMDLINE_PARTS is set to "m" I think MTD_RESERVE_END will not be set.
In that case CPP will helpfully set CONFIG_MTD_RESERVE_END to zero
(which is what you want). But it should also trigger this warning:
    "CONFIG_MTD_RESERVE_END" is not defined [-Wundef]

>  
>  		if (offset + part->parts[i].size > master->size) {
>  			printk(KERN_WARNING ERRP


Paul Bolle

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