Re: [PATCH] env: Make _init() expect _INVALID when _IS_NOWHERE
On Fri, Sep 24, 2021 at 05:04:27AM +0200, Marek Vasut wrote: > On 9/24/21 4:40 AM, Tom Rini wrote: > > On Thu, Aug 12, 2021 at 03:28:31PM +, Pierre-Clément Tosi wrote: > > > > > Avoid applying the "fix" introduced by commit 5557eec01cbf ("env: Fix > > > invalid env handling in env_init()") to the environment "nowhere". > > > > > > This is necessary as that commit, by setting the return value of > > > env_init() to -ENOENT if gd->env_valid is ENV_INVALID, forces that > > > function to reset gd->env_valid to ENV_VALID. By doing so, it breaks the > > > assumption (required by ENV_IS_NOWHERE) that gd->env_valid must be > > > ENV_INVALID. > > > > > > This, in turn, results in env_relocate() calling env_load() (it should > > > not), which itself, calls U_BOOT_ENV_LOCATION(nowhere).load() i.e. > > > env_nowhere_load(). That function, being implemented under the > > > assumption mentioned above, calls env_set_default(), which in turn, > > > seeing that gd->env_valid is ENV_VALID (it should not), tries to > > > dereference whatever lies in gd->env_addr (most likely garbage), leading > > > to a faulty memory access. > > > > > > Note that other env_locations might be concerned by this bug but that > > > this commit only intends to fix it for when ENV_IS_NOWHERE. > > > > > > Fixes: 5557eec01cbf ("env: Fix invalid env handling in env_init()") > > > Signed-off-by: Pierre-Clément Tosi > > > > Applied to u-boot/next, thanks! > > Please drop this, quote from my previous reply to this patch: > > " > Such patch was already posted, see the patch and the discussion, in the end > there was a different fix for the relocation code: > > https://patchwork.ozlabs.org/project/uboot/patch/1620828554-24013-1-git-send-email-hayashi.kunih...@socionext.com/ > " I'll drop this. FWIW, my reading of the previous patch, and then this being posted afterwards is why I had applied this patch. -- Tom signature.asc Description: PGP signature
Re: [PATCH] env: Make _init() expect _INVALID when _IS_NOWHERE
On 9/24/21 4:40 AM, Tom Rini wrote: On Thu, Aug 12, 2021 at 03:28:31PM +, Pierre-Clément Tosi wrote: Avoid applying the "fix" introduced by commit 5557eec01cbf ("env: Fix invalid env handling in env_init()") to the environment "nowhere". This is necessary as that commit, by setting the return value of env_init() to -ENOENT if gd->env_valid is ENV_INVALID, forces that function to reset gd->env_valid to ENV_VALID. By doing so, it breaks the assumption (required by ENV_IS_NOWHERE) that gd->env_valid must be ENV_INVALID. This, in turn, results in env_relocate() calling env_load() (it should not), which itself, calls U_BOOT_ENV_LOCATION(nowhere).load() i.e. env_nowhere_load(). That function, being implemented under the assumption mentioned above, calls env_set_default(), which in turn, seeing that gd->env_valid is ENV_VALID (it should not), tries to dereference whatever lies in gd->env_addr (most likely garbage), leading to a faulty memory access. Note that other env_locations might be concerned by this bug but that this commit only intends to fix it for when ENV_IS_NOWHERE. Fixes: 5557eec01cbf ("env: Fix invalid env handling in env_init()") Signed-off-by: Pierre-Clément Tosi Applied to u-boot/next, thanks! Please drop this, quote from my previous reply to this patch: " Such patch was already posted, see the patch and the discussion, in the end there was a different fix for the relocation code: https://patchwork.ozlabs.org/project/uboot/patch/1620828554-24013-1-git-send-email-hayashi.kunih...@socionext.com/ "
Re: [PATCH] env: Make _init() expect _INVALID when _IS_NOWHERE
On Thu, Aug 12, 2021 at 03:28:31PM +, Pierre-Clément Tosi wrote: > Avoid applying the "fix" introduced by commit 5557eec01cbf ("env: Fix > invalid env handling in env_init()") to the environment "nowhere". > > This is necessary as that commit, by setting the return value of > env_init() to -ENOENT if gd->env_valid is ENV_INVALID, forces that > function to reset gd->env_valid to ENV_VALID. By doing so, it breaks the > assumption (required by ENV_IS_NOWHERE) that gd->env_valid must be > ENV_INVALID. > > This, in turn, results in env_relocate() calling env_load() (it should > not), which itself, calls U_BOOT_ENV_LOCATION(nowhere).load() i.e. > env_nowhere_load(). That function, being implemented under the > assumption mentioned above, calls env_set_default(), which in turn, > seeing that gd->env_valid is ENV_VALID (it should not), tries to > dereference whatever lies in gd->env_addr (most likely garbage), leading > to a faulty memory access. > > Note that other env_locations might be concerned by this bug but that > this commit only intends to fix it for when ENV_IS_NOWHERE. > > Fixes: 5557eec01cbf ("env: Fix invalid env handling in env_init()") > Signed-off-by: Pierre-Clément Tosi Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
[PATCH] env: Make _init() expect _INVALID when _IS_NOWHERE
Avoid applying the "fix" introduced by commit 5557eec01cbf ("env: Fix invalid env handling in env_init()") to the environment "nowhere". This is necessary as that commit, by setting the return value of env_init() to -ENOENT if gd->env_valid is ENV_INVALID, forces that function to reset gd->env_valid to ENV_VALID. By doing so, it breaks the assumption (required by ENV_IS_NOWHERE) that gd->env_valid must be ENV_INVALID. This, in turn, results in env_relocate() calling env_load() (it should not), which itself, calls U_BOOT_ENV_LOCATION(nowhere).load() i.e. env_nowhere_load(). That function, being implemented under the assumption mentioned above, calls env_set_default(), which in turn, seeing that gd->env_valid is ENV_VALID (it should not), tries to dereference whatever lies in gd->env_addr (most likely garbage), leading to a faulty memory access. Note that other env_locations might be concerned by this bug but that this commit only intends to fix it for when ENV_IS_NOWHERE. Fixes: 5557eec01cbf ("env: Fix invalid env handling in env_init()") Signed-off-by: Pierre-Clément Tosi --- env/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/env.c b/env/env.c index e534008006..0a0f234747 100644 --- a/env/env.c +++ b/env/env.c @@ -336,7 +336,7 @@ int env_init(void) debug("%s: Environment %s init done (ret=%d)\n", __func__, drv->name, ret); - if (gd->env_valid == ENV_INVALID) + if (gd->env_valid == ENV_INVALID && drv->location != ENVL_NOWHERE) ret = -ENOENT; } -- 2.32.0.605.g8dce9f2422-goog -- Pierre
Re: [PATCH] env: Make _init() expect _INVALID when _IS_NOWHERE
On 8/12/21 5:28 PM, Pierre-Clément Tosi wrote: Avoid applying the "fix" introduced by commit 5557eec01cbf ("env: Fix invalid env handling in env_init()") to the environment "nowhere". This is necessary as that commit, by setting the return value of env_init() to -ENOENT if gd->env_valid is ENV_INVALID, forces that function to reset gd->env_valid to ENV_VALID. By doing so, it breaks the assumption (required by ENV_IS_NOWHERE) that gd->env_valid must be ENV_INVALID. This, in turn, results in env_relocate() calling env_load() (it should not), which itself, calls U_BOOT_ENV_LOCATION(nowhere).load() i.e. env_nowhere_load(). That function, being implemented under the assumption mentioned above, calls env_set_default(), which in turn, seeing that gd->env_valid is ENV_VALID (it should not), tries to dereference whatever lies in gd->env_addr (most likely garbage), leading to a faulty memory access. Note that other env_locations might be concerned by this bug but that this commit only intends to fix it for when ENV_IS_NOWHERE. Fixes: 5557eec01cbf ("env: Fix invalid env handling in env_init()") Signed-off-by: Pierre-Clément Tosi Such patch was already posted, see the patch and the discussion, in the end there was a different fix for the relocation code: https://patchwork.ozlabs.org/project/uboot/patch/1620828554-24013-1-git-send-email-hayashi.kunih...@socionext.com/