All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Protsenko <semen.protsenko@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 1/3] dfu: Fix data abort in dfu_free_entities()
Date: Tue, 24 Jul 2018 19:41:12 +0300	[thread overview]
Message-ID: <CAKaJLVvVc+CLfB=qsHJaOEbjrfTc1ZeO_UkDYL5BRMOTRu=RDQ@mail.gmail.com> (raw)
In-Reply-To: <20180713133547.21094-1-semen.protsenko@linaro.org>

On Fri, Jul 13, 2018 at 4:35 PM, Sam Protsenko
<semen.protsenko@linaro.org> wrote:
> Commit 5d8fae79163e ("dfu: avoid memory leak") brings a regression which
> described below. This patch is effectively reverting that commit, adding
> corresponding comment to avoid such regressions in future.
>
> In case of error in dfu_config_entities(), it frees "dfu" array, which
> leads to "data abort" in dfu_free_entities(), which tries to free the
> same array (and even tries to access it from linked list first). The
> issue occurs e.g. when partition table on device does not match
> $dfu_alt_info layout:
>
>     => dfu 0 mmc 1
>     Couldn't find part #2 on mmc device #1
>     DFU entities configuration failed!
>     data abort
>
> To fix this issue, do not free "dfu" array in dfu_config_entities(). It
> will be freed later in dfu_free_entities().
>
> Tested on BeagleBone Black (where this regression was originally found).
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v2:
>   - Improve commit message by mentioning regression commit
>

Hi Lukasz,

Can you please review and merge this whole series?

Thanks!

>  drivers/dfu/dfu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index e7c91193b9..a3c09334b7 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -462,7 +462,7 @@ int dfu_config_entities(char *env, char *interface, char *devstr)
>                 ret = dfu_fill_entity(&dfu[i], s, alt_num_cnt, interface,
>                                       devstr);
>                 if (ret) {
> -                       free(dfu);
> +                       /* We will free "dfu" in dfu_free_entities() */
>                         return -1;
>                 }
>
> --
> 2.18.0
>

  parent reply	other threads:[~2018-07-24 16:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-13 13:35 [U-Boot] [PATCH v2 1/3] dfu: Fix data abort in dfu_free_entities() Sam Protsenko
2018-07-13 13:35 ` [U-Boot] [PATCH v2 2/3] dfu: Fix memory leak in dfu_init_env_entities() Sam Protsenko
2018-07-13 13:35 ` [U-Boot] [PATCH v2 3/3] dfu: Provide more verbose error message Sam Protsenko
2018-07-24 16:41 ` Sam Protsenko [this message]
2018-07-24 21:14   ` [U-Boot] [PATCH v2 1/3] dfu: Fix data abort in dfu_free_entities() Lukasz Majewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKaJLVvVc+CLfB=qsHJaOEbjrfTc1ZeO_UkDYL5BRMOTRu=RDQ@mail.gmail.com' \
    --to=semen.protsenko@linaro.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.