linux-kernel - Re: [PATCH] UBI: allow atomic updates to sychronously erase old PEB

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: <1334147205.13842.8.camel@sauron.fi.intel.com>
Date:	Wed, 11 Apr 2012 15:26:45 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Joel Reardon <joel@...mbassador.com>
Cc:	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] UBI: allow atomic updates to sychronously erase old PEB

Hi,

sorry for long delay.

First of all - please, make UBI and UBIFS changes in separate patches -
we consider these to be separate subsystems.

On Fri, 2012-03-30 at 17:11 +0200, Joel Reardon wrote:
> @@ -629,12 +628,13 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
>   * @ubi: UBI device description object
>   * @e: the WL entry of the physical eraseblock to erase
>   * @torture: if the physical eraseblock has to be tortured
> + * @sync: schedule the work immediately and return after completion
>   *
>   * This function returns zero in case of success and a %-ENOMEM in case of
>   * failure.
>   */
>  static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
> -			  int torture)
> +			  int torture, int sync)
>  {
>  	struct ubi_work *wl_wrk;
> 
> @@ -649,7 +649,11 @@ static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
>  	wl_wrk->e = e;
>  	wl_wrk->torture = torture;
> 
> -	schedule_ubi_work(ubi, wl_wrk);
> +	if (sync)
> +		erase_worker(ubi, wl_wrk, 0);
> +	else
> +		schedule_ubi_work(ubi, wl_wrk);
> +
>  	return 0;
>  }

Please, do not modify this function. You only need to do "if (sync)"
in 'ubi_wl_put_peb()' and nowhere else, so do it directly there.

You will have no "what if sync fails" issues then. Also, just from
common sense point of view, "schedule_erase" should schedule, not erase.

Otherwise looks good - I can take this to the ubifs tree straight
away even without your security stuff.

-- 
Best Regards,
Artem Bityutskiy

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)