linux-kernel - Re: [PATCH] [RFC] UBI: Implement Fastmap support

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: <1337694184.2483.186.camel@sauron.fi.intel.com>
Date:	Tue, 22 May 2012 16:43:04 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Richard Weinberger <richard@....at>
Cc:	linux-mtd@...ts.infradead.org, tglx@...utronix.de,
	linux-kernel@...r.kernel.org, Heinz.Egger@...utronix.de,
	tim.bird@...sony.com
Subject: Re: [PATCH] [RFC] UBI: Implement Fastmap support

Uhh, too many helpers.

On Mon, 2012-05-21 at 16:01 +0200, Richard Weinberger wrote:
>  /**
> - * ubi_attach - attach an MTD device.
> - * @ubi: UBI device descriptor
> + * scan_fastmap - attach MTD device using fastmap.
> + * @ubi: UBI device description object
>   *
> - * This function returns zero in case of success and a negative error code in
> - * case of failure.
> + * This function attaches a MTD device using a fastmap and returns complete
> + * information about it in form of a "struct ubi_attach_info" object. In case
> + * of failure, an error code is returned.
>   */
> -int ubi_attach(struct ubi_device *ubi)
> +static struct ubi_attach_info *scan_fastmap(struct ubi_device *ubi)
> +{
> +       int fm_start;
> +
> +       fm_start = ubi_find_fastmap(ubi);
> +       if (fm_start < 0)
> +               return ERR_PTR(-ENOENT);
> +
> +       return ubi_read_fastmap(ubi, fm_start);
> +}

This helper which does not do anything useful should not exist - just
teach 'ubi_read_fastmap()' to return 1 if the fastmap is not found, and
< 0 on error, and 0 on success. No one in attach.c should be interested
in fm_start. So this helper should die. See below as well.

> +
> +static int do_attach(struct ubi_device *ubi, bool fullscan)
>  {
>         int err;
>         struct ubi_attach_info *ai;
>  
> -       ai = scan_all(ubi);
> +       if (fullscan)
> +               ai = scan_all(ubi);
> +       else
> +               ai = scan_fastmap(ubi);
> +
>         if (IS_ERR(ai))
>                 return PTR_ERR(ai);
>  
> @@ -1256,6 +1277,31 @@ out_ai:
>  }

Another useless helper function, should die, see below.

>  
>  /**
> + * ubi_attach - attach an MTD device.
> + * @ubi: UBI device descriptor
> + *
> + * This function returns zero in case of success and a negative error code in
> + * case of failure.
> + */
> +int ubi_attach(struct ubi_device *ubi)
> +{
> +       int err;
> +
> +       err = do_attach(ubi, false);
> +       if (err) {
> +               if (err != -ENOENT)
> +                       ubi_err("Attach by fastmap failed! " \
> +                               "Falling back to attach by scanning. " \
> +                               "error = %i\n", err);
> +
> +               ubi->attached_by_scanning = true;
> +               err = do_attach(ubi, true);
> +       }
> +
> +       return err;
> +} 

Just add this code to this function:

       err = ubi_read_fastmap();
       if (err < 0)
               return err;
       if (err == 2)
               return scan_all();

       ... rest of the common nitializations (EBA, WL)...

       return err;

-- 
Best Regards,
Artem Bityutskiy

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