| Message ID | 20200729193959.23115-1-liu.ming50@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [libubootenv] uboot_env: Use canonicalized pathname when reading device | expand |
Hi Ming, On 29.07.20 21:39, liu.ming50@gmail.com wrote: > From: Ming Liu <liu.ming50@gmail.com> > > Some platform uses softlinks to the devices that hold environment > data. The mechanism used to read device type from config is not robust > in this case. Calculating the canonicalized absolute pathname of the > device could fix the problem. > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com> > Signed-off-by: Ming Liu <liu.ming50@gmail.com> > --- > src/uboot_env.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/uboot_env.c b/src/uboot_env.c > index 25de4fb..aceaa98 100644 > --- a/src/uboot_env.c > +++ b/src/uboot_env.c > @@ -20,6 +20,7 @@ > #include <stddef.h> > #include <dirent.h> > #include <unistd.h> > +#include <limits.h> > #include <linux/fs.h> > #include <string.h> > #include <fcntl.h> > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct uboot_ctx *ctx, const char *config) > int ndev = 0; > struct uboot_flash_env *dev; > char *tmp; > + char *path; > int retval = 0; > > if (!config) > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct uboot_ctx *ctx, const char *config) > ctx->size = dev->envsize; > > if (tmp) { > + if ((path = realpath(tmp, NULL)) == NULL) { > + free(tmp); > + retval = -EINVAL; > + break; > + } > memset(dev->devname, 0, sizeof(dev->devname)); > - strncpy(dev->devname, tmp, sizeof(dev->devname) - 1); > + strncpy(dev->devname, path, sizeof(dev->devname) - 1); > free(tmp); > + free(path); > } > > if (check_env_device(ctx, dev) < 0) { > Looks good. Reviewed-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Thanks for the review, please hold on merging, I seems found a issue with this patch, will need more test and probably will send a V2. //Ming Liu Hi Ming, > > On 29.07.20 21:39, liu.ming50@gmail.com wrote: > > From: Ming Liu <liu.ming50@gmail.com> > > > > Some platform uses softlinks to the devices that hold environment > > data. The mechanism used to read device type from config is not robust > > in this case. Calculating the canonicalized absolute pathname of the > > device could fix the problem. > > > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com> > > Signed-off-by: Ming Liu <liu.ming50@gmail.com> > > --- > > src/uboot_env.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/src/uboot_env.c b/src/uboot_env.c > > index 25de4fb..aceaa98 100644 > > --- a/src/uboot_env.c > > +++ b/src/uboot_env.c > > @@ -20,6 +20,7 @@ > > #include <stddef.h> > > #include <dirent.h> > > #include <unistd.h> > > +#include <limits.h> > > #include <linux/fs.h> > > #include <string.h> > > #include <fcntl.h> > > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct uboot_ctx *ctx, > const char *config) > > int ndev = 0; > > struct uboot_flash_env *dev; > > char *tmp; > > + char *path; > > int retval = 0; > > > > if (!config) > > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct uboot_ctx *ctx, > const char *config) > > ctx->size = dev->envsize; > > > > if (tmp) { > > + if ((path = realpath(tmp, NULL)) == NULL) { > > + free(tmp); > > + retval = -EINVAL; > > + break; > > + } > > memset(dev->devname, 0, sizeof(dev->devname)); > > - strncpy(dev->devname, tmp, sizeof(dev->devname) - > 1); > > + strncpy(dev->devname, path, sizeof(dev->devname) - > 1); > > free(tmp); > > + free(path); > > } > > > > if (check_env_device(ctx, dev) < 0) { > > > > Looks good. > > Reviewed-by: Stefano Babic <sbabic@denx.de> > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ===================================================================== >
On 30.07.20 12:51, Ming Liu wrote: > Thanks for the review, please hold on merging, I seems found a issue > with this patch, will need more test and probably will send a V2. > ok, thanks. Regards, Stefano > //Ming Liu > > > Hi Ming, > > On 29.07.20 21:39, liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> wrote: > > From: Ming Liu <liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>> > > > > Some platform uses softlinks to the devices that hold environment > > data. The mechanism used to read device type from config is not robust > > in this case. Calculating the canonicalized absolute pathname of the > > device could fix the problem. > > > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com > <mailto:mathias.thore@atlascopco.com>> > > Signed-off-by: Ming Liu <liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com>> > > --- > > src/uboot_env.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/src/uboot_env.c b/src/uboot_env.c > > index 25de4fb..aceaa98 100644 > > --- a/src/uboot_env.c > > +++ b/src/uboot_env.c > > @@ -20,6 +20,7 @@ > > #include <stddef.h> > > #include <dirent.h> > > #include <unistd.h> > > +#include <limits.h> > > #include <linux/fs.h> > > #include <string.h> > > #include <fcntl.h> > > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct uboot_ctx > *ctx, const char *config) > > int ndev = 0; > > struct uboot_flash_env *dev; > > char *tmp; > > + char *path; > > int retval = 0; > > > > if (!config) > > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct uboot_ctx > *ctx, const char *config) > > ctx->size = dev->envsize; > > > > if (tmp) { > > + if ((path = realpath(tmp, NULL)) == NULL) { > > + free(tmp); > > + retval = -EINVAL; > > + break; > > + } > > memset(dev->devname, 0, sizeof(dev->devname)); > > - strncpy(dev->devname, tmp, > sizeof(dev->devname) - 1); > > + strncpy(dev->devname, path, > sizeof(dev->devname) - 1); > > free(tmp); > > + free(path); > > } > > > > if (check_env_device(ctx, dev) < 0) { > > > > Looks good. > > Reviewed-by: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > sbabic@denx.de <mailto:sbabic@denx.de> > ===================================================================== >
I have done more tests, it's sort of strange, the problem I mentioned is
not caused by my patch, but the current libubootenv on:
```
commit 4a0a4662e25471e8fcaf8767a4fa4f454c88a2e2
Merge: 6117831 41b5188
Author: Stefano Babic <sbabic@denx.de>
Date: Thu Jul 2 14:37:39 2020 +0200
Merge pull request #7 from TomzBench/cmake-fix
fixed install for static target, fixed BUILD_DOC acknowledgement
```
does not work on my imx7 machine, with the following fw_env.config:
```
# Block device name Device offset Env. size
/dev/mmcblk0boot0 -0x2200 0x2000
```
but the commit
```
commit 86bd30a14e153a18f670b25708795253d8736f0f
Author: Kristian Amlie <kristian.amlie@northern.tech>
Date: Mon Jun 22 14:06:24 2020 +0200
Restore ability to feed script file via stdin, using `-s -`.
This used to work in the fw-utils based on U-Boot itself.
Signed-off-by: Kristian Amlie <kristian.amlie@northern.tech>
Reviewed-by: Stefano Babic <sbabic@denx.de>
```
does work, this is the srcrev being used in OE, maybe we are having a
regression in the latest libubootenv?
//Ming Liu
Stefano Babic <sbabic@denx.de> 於 2020年7月30日 週四 下午12:52寫道：
> On 30.07.20 12:51, Ming Liu wrote:
> > Thanks for the review, please hold on merging, I seems found a issue
> > with this patch, will need more test and probably will send a V2.
> >
>
> ok, thanks.
>
> Regards,
> Stefano
>
> > //Ming Liu
> >
> >
> > Hi Ming,
> >
> > On 29.07.20 21:39, liu.ming50@gmail.com
> > <mailto:liu.ming50@gmail.com> wrote:
> > > From: Ming Liu <liu.ming50@gmail.com <mailto:liu.ming50@gmail.com
> >>
> > >
> > > Some platform uses softlinks to the devices that hold environment
> > > data. The mechanism used to read device type from config is not
> robust
> > > in this case. Calculating the canonicalized absolute pathname of
> the
> > > device could fix the problem.
> > >
> > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com
> > <mailto:mathias.thore@atlascopco.com>>
> > > Signed-off-by: Ming Liu <liu.ming50@gmail.com
> > <mailto:liu.ming50@gmail.com>>
> > > ---
> > > src/uboot_env.c | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/uboot_env.c b/src/uboot_env.c
> > > index 25de4fb..aceaa98 100644
> > > --- a/src/uboot_env.c
> > > +++ b/src/uboot_env.c
> > > @@ -20,6 +20,7 @@
> > > #include <stddef.h>
> > > #include <dirent.h>
> > > #include <unistd.h>
> > > +#include <limits.h>
> > > #include <linux/fs.h>
> > > #include <string.h>
> > > #include <fcntl.h>
> > > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct uboot_ctx
> > *ctx, const char *config)
> > > int ndev = 0;
> > > struct uboot_flash_env *dev;
> > > char *tmp;
> > > + char *path;
> > > int retval = 0;
> > >
> > > if (!config)
> > > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct uboot_ctx
> > *ctx, const char *config)
> > > ctx->size = dev->envsize;
> > >
> > > if (tmp) {
> > > + if ((path = realpath(tmp, NULL)) == NULL) {
> > > + free(tmp);
> > > + retval = -EINVAL;
> > > + break;
> > > + }
> > > memset(dev->devname, 0,
> sizeof(dev->devname));
> > > - strncpy(dev->devname, tmp,
> > sizeof(dev->devname) - 1);
> > > + strncpy(dev->devname, path,
> > sizeof(dev->devname) - 1);
> > > free(tmp);
> > > + free(path);
> > > }
> > >
> > > if (check_env_device(ctx, dev) < 0) {
> > >
> >
> > Looks good.
> >
> > Reviewed-by: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>>
> >
> > Best regards,
> > Stefano Babic
> >
> > --
> > =====================================================================
> > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email:
> > sbabic@denx.de <mailto:sbabic@denx.de>
> > =====================================================================
> >
>
>
> --
> =====================================================================
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> =====================================================================
>
Hi Ming, On 03.08.20 12:18, Ming Liu wrote: > I have done more tests, it's sort of strange, the problem I mentioned is > not caused by my patch, but the current libubootenv on: > ``` > commit 4a0a4662e25471e8fcaf8767a4fa4f454c88a2e2 > Merge: 6117831 41b5188 > Author: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> > Date: Thu Jul 2 14:37:39 2020 +0200 > > Merge pull request #7 from TomzBench/cmake-fix > > fixed install for static target, fixed BUILD_DOC acknowledgement > ``` > > does not work on my imx7 machine, with the following fw_env.config: > ``` > # Block device name Device offset Env. size > /dev/mmcblk0boot0 -0x2200 0x2000 > ``` > > but the commit > ``` > commit 86bd30a14e153a18f670b25708795253d8736f0f > Author: Kristian Amlie <kristian.amlie@northern.tech> > Date: Mon Jun 22 14:06:24 2020 +0200 > > Restore ability to feed script file via stdin, using `-s -`. > > This used to work in the fw-utils based on U-Boot itself. > > Signed-off-by: Kristian Amlie <kristian.amlie@northern.tech> > Reviewed-by: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> > ``` > does work, this is the srcrev being used in OE, maybe we are having a > regression in the latest libubootenv? Please run git-bisect to find out which commit is the cause. Best regards, Stefano Babic > > //Ming Liu > > Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> 於 2020年7月30日 > 週四 下午12:52寫道： > > On 30.07.20 12:51, Ming Liu wrote: > > Thanks for the review, please hold on merging, I seems found a issue > > with this patch, will need more test and probably will send a V2. > > > > ok, thanks. > > Regards, > Stefano > > > //Ming Liu > > > > > > Hi Ming, > > > > On 29.07.20 21:39, liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>> wrote: > > > From: Ming Liu <liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com>>> > > > > > > Some platform uses softlinks to the devices that hold > environment > > > data. The mechanism used to read device type from config is > not robust > > > in this case. Calculating the canonicalized absolute > pathname of the > > > device could fix the problem. > > > > > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com > <mailto:mathias.thore@atlascopco.com> > > <mailto:mathias.thore@atlascopco.com > <mailto:mathias.thore@atlascopco.com>>> > > > Signed-off-by: Ming Liu <liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>>> > > > --- > > > src/uboot_env.c | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/uboot_env.c b/src/uboot_env.c > > > index 25de4fb..aceaa98 100644 > > > --- a/src/uboot_env.c > > > +++ b/src/uboot_env.c > > > @@ -20,6 +20,7 @@ > > > #include <stddef.h> > > > #include <dirent.h> > > > #include <unistd.h> > > > +#include <limits.h> > > > #include <linux/fs.h> > > > #include <string.h> > > > #include <fcntl.h> > > > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct uboot_ctx > > *ctx, const char *config) > > > int ndev = 0; > > > struct uboot_flash_env *dev; > > > char *tmp; > > > + char *path; > > > int retval = 0; > > > > > > if (!config) > > > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct uboot_ctx > > *ctx, const char *config) > > > ctx->size = dev->envsize; > > > > > > if (tmp) { > > > + if ((path = realpath(tmp, NULL)) == > NULL) { > > > + free(tmp); > > > + retval = -EINVAL; > > > + break; > > > + } > > > memset(dev->devname, 0, > sizeof(dev->devname)); > > > - strncpy(dev->devname, tmp, > > sizeof(dev->devname) - 1); > > > + strncpy(dev->devname, path, > > sizeof(dev->devname) - 1); > > > free(tmp); > > > + free(path); > > > } > > > > > > if (check_env_device(ctx, dev) < 0) { > > > > > > > Looks good. > > > > Reviewed-by: Stefano Babic <sbabic@denx.de > <mailto:sbabic@denx.de> <mailto:sbabic@denx.de <mailto:sbabic@denx.de>>> > > > > Best regards, > > Stefano Babic > > > > -- > > > ===================================================================== > > DENX Software Engineering GmbH, Managing Director: > Wolfgang Denk > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > Germany > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > > sbabic@denx.de <mailto:sbabic@denx.de> <mailto:sbabic@denx.de > <mailto:sbabic@denx.de>> > > > ===================================================================== > > > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > sbabic@denx.de <mailto:sbabic@denx.de> > ===================================================================== > > -- > You received this message because you are subscribed to the Google > Groups "swupdate" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to swupdate+unsubscribe@googlegroups.com > <mailto:swupdate+unsubscribe@googlegroups.com>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com > <https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com?utm_medium=email&utm_source=footer>.
Sure, will do git bisect. //Ming Liu Stefano Babic <sbabic@denx.de> 於 2020年8月3日 週一 下午12:25寫道： > Hi Ming, > > On 03.08.20 12:18, Ming Liu wrote: > > I have done more tests, it's sort of strange, the problem I mentioned is > > not caused by my patch, but the current libubootenv on: > > ``` > > commit 4a0a4662e25471e8fcaf8767a4fa4f454c88a2e2 > > Merge: 6117831 41b5188 > > Author: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> > > Date: Thu Jul 2 14:37:39 2020 +0200 > > > > Merge pull request #7 from TomzBench/cmake-fix > > > > fixed install for static target, fixed BUILD_DOC acknowledgement > > ``` > > > > does not work on my imx7 machine, with the following fw_env.config: > > ``` > > # Block device name Device offset Env. size > > /dev/mmcblk0boot0 -0x2200 0x2000 > > ``` > > > > but the commit > > ``` > > commit 86bd30a14e153a18f670b25708795253d8736f0f > > Author: Kristian Amlie <kristian.amlie@northern.tech> > > Date: Mon Jun 22 14:06:24 2020 +0200 > > > > Restore ability to feed script file via stdin, using `-s -`. > > > > This used to work in the fw-utils based on U-Boot itself. > > > > Signed-off-by: Kristian Amlie <kristian.amlie@northern.tech> > > Reviewed-by: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> > > ``` > > does work, this is the srcrev being used in OE, maybe we are having a > > regression in the latest libubootenv? > > Please run git-bisect to find out which commit is the cause. > > Best regards, > Stefano Babic > > > > > > //Ming Liu > > > > Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> 於 2020年7月30日 > > 週四 下午12:52寫道： > > > > On 30.07.20 12:51, Ming Liu wrote: > > > Thanks for the review, please hold on merging, I seems found a > issue > > > with this patch, will need more test and probably will send a V2. > > > > > > > ok, thanks. > > > > Regards, > > Stefano > > > > > //Ming Liu > > > > > > > > > Hi Ming, > > > > > > On 29.07.20 21:39, liu.ming50@gmail.com > > <mailto:liu.ming50@gmail.com> > > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>> > wrote: > > > > From: Ming Liu <liu.ming50@gmail.com > > <mailto:liu.ming50@gmail.com> <mailto:liu.ming50@gmail.com > > <mailto:liu.ming50@gmail.com>>> > > > > > > > > Some platform uses softlinks to the devices that hold > > environment > > > > data. The mechanism used to read device type from config is > > not robust > > > > in this case. Calculating the canonicalized absolute > > pathname of the > > > > device could fix the problem. > > > > > > > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com > > <mailto:mathias.thore@atlascopco.com> > > > <mailto:mathias.thore@atlascopco.com > > <mailto:mathias.thore@atlascopco.com>>> > > > > Signed-off-by: Ming Liu <liu.ming50@gmail.com > > <mailto:liu.ming50@gmail.com> > > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>>> > > > > --- > > > > src/uboot_env.c | 10 +++++++++- > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/uboot_env.c b/src/uboot_env.c > > > > index 25de4fb..aceaa98 100644 > > > > --- a/src/uboot_env.c > > > > +++ b/src/uboot_env.c > > > > @@ -20,6 +20,7 @@ > > > > #include <stddef.h> > > > > #include <dirent.h> > > > > #include <unistd.h> > > > > +#include <limits.h> > > > > #include <linux/fs.h> > > > > #include <string.h> > > > > #include <fcntl.h> > > > > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct > uboot_ctx > > > *ctx, const char *config) > > > > int ndev = 0; > > > > struct uboot_flash_env *dev; > > > > char *tmp; > > > > + char *path; > > > > int retval = 0; > > > > > > > > if (!config) > > > > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct > uboot_ctx > > > *ctx, const char *config) > > > > ctx->size = dev->envsize; > > > > > > > > if (tmp) { > > > > + if ((path = realpath(tmp, NULL)) == > > NULL) { > > > > + free(tmp); > > > > + retval = -EINVAL; > > > > + break; > > > > + } > > > > memset(dev->devname, 0, > > sizeof(dev->devname)); > > > > - strncpy(dev->devname, tmp, > > > sizeof(dev->devname) - 1); > > > > + strncpy(dev->devname, path, > > > sizeof(dev->devname) - 1); > > > > free(tmp); > > > > + free(path); > > > > } > > > > > > > > if (check_env_device(ctx, dev) < 0) { > > > > > > > > > > Looks good. > > > > > > Reviewed-by: Stefano Babic <sbabic@denx.de > > <mailto:sbabic@denx.de> <mailto:sbabic@denx.de <mailto: > sbabic@denx.de>>> > > > > > > Best regards, > > > Stefano Babic > > > > > > -- > > > > > > ===================================================================== > > > DENX Software Engineering GmbH, Managing Director: > > Wolfgang Denk > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > > Germany > > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > > > sbabic@denx.de <mailto:sbabic@denx.de> <mailto:sbabic@denx.de > > <mailto:sbabic@denx.de>> > > > > > > ===================================================================== > > > > > > > > > -- > > ===================================================================== > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > > sbabic@denx.de <mailto:sbabic@denx.de> > > ===================================================================== > > > > -- > > You received this message because you are subscribed to the Google > > Groups "swupdate" group. > > To unsubscribe from this group and stop receiving emails from it, send > > an email to swupdate+unsubscribe@googlegroups.com > > <mailto:swupdate+unsubscribe@googlegroups.com>. > > To view this discussion on the web visit > > > https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com > > < > https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com?utm_medium=email&utm_source=footer > >. > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ===================================================================== >
Hi, Stefano:
I think the issue I mentioned has been fixed by the following commit:
```
Fix bug introduced by commit 52a70114
The return value from lseek is not checked correctly, causing that the
environment is not read or written if an offset is set.
Signed-off-by: Stefano Babic <sbabic@denx.de>
```
will send a V2 of this patch.
//Ming Liu
Ming Liu <liu.ming50@gmail.com> 於 2020年8月3日 週一 下午12:43寫道：
> Sure, will do git bisect.
>
> //Ming Liu
>
> Stefano Babic <sbabic@denx.de> 於 2020年8月3日 週一 下午12:25寫道：
>
>> Hi Ming,
>>
>> On 03.08.20 12:18, Ming Liu wrote:
>> > I have done more tests, it's sort of strange, the problem I mentioned is
>> > not caused by my patch, but the current libubootenv on:
>> > ```
>> > commit 4a0a4662e25471e8fcaf8767a4fa4f454c88a2e2
>> > Merge: 6117831 41b5188
>> > Author: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>>
>> > Date: Thu Jul 2 14:37:39 2020 +0200
>> >
>> > Merge pull request #7 from TomzBench/cmake-fix
>> >
>> > fixed install for static target, fixed BUILD_DOC acknowledgement
>> > ```
>> >
>> > does not work on my imx7 machine, with the following fw_env.config:
>> > ```
>> > # Block device name Device offset Env. size
>> > /dev/mmcblk0boot0 -0x2200 0x2000
>> > ```
>> >
>> > but the commit
>> > ```
>> > commit 86bd30a14e153a18f670b25708795253d8736f0f
>> > Author: Kristian Amlie <kristian.amlie@northern.tech>
>> > Date: Mon Jun 22 14:06:24 2020 +0200
>> >
>> > Restore ability to feed script file via stdin, using `-s -`.
>> >
>> > This used to work in the fw-utils based on U-Boot itself.
>> >
>> > Signed-off-by: Kristian Amlie <kristian.amlie@northern.tech>
>> > Reviewed-by: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>>
>> > ```
>> > does work, this is the srcrev being used in OE, maybe we are having a
>> > regression in the latest libubootenv?
>>
>> Please run git-bisect to find out which commit is the cause.
>>
>> Best regards,
>> Stefano Babic
>>
>>
>> >
>> > //Ming Liu
>> >
>> > Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> 於 2020年7月30日
>> > 週四 下午12:52寫道：
>> >
>> > On 30.07.20 12:51, Ming Liu wrote:
>> > > Thanks for the review, please hold on merging, I seems found a
>> issue
>> > > with this patch, will need more test and probably will send a V2.
>> > >
>> >
>> > ok, thanks.
>> >
>> > Regards,
>> > Stefano
>> >
>> > > //Ming Liu
>> > >
>> > >
>> > > Hi Ming,
>> > >
>> > > On 29.07.20 21:39, liu.ming50@gmail.com
>> > <mailto:liu.ming50@gmail.com>
>> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>>
>> wrote:
>> > > > From: Ming Liu <liu.ming50@gmail.com
>> > <mailto:liu.ming50@gmail.com> <mailto:liu.ming50@gmail.com
>> > <mailto:liu.ming50@gmail.com>>>
>> > > >
>> > > > Some platform uses softlinks to the devices that hold
>> > environment
>> > > > data. The mechanism used to read device type from config is
>> > not robust
>> > > > in this case. Calculating the canonicalized absolute
>> > pathname of the
>> > > > device could fix the problem.
>> > > >
>> > > > Signed-off-by: Mathias Thore <mathias.thore@atlascopco.com
>> > <mailto:mathias.thore@atlascopco.com>
>> > > <mailto:mathias.thore@atlascopco.com
>> > <mailto:mathias.thore@atlascopco.com>>>
>> > > > Signed-off-by: Ming Liu <liu.ming50@gmail.com
>> > <mailto:liu.ming50@gmail.com>
>> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>>>
>> > > > ---
>> > > > src/uboot_env.c | 10 +++++++++-
>> > > > 1 file changed, 9 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/src/uboot_env.c b/src/uboot_env.c
>> > > > index 25de4fb..aceaa98 100644
>> > > > --- a/src/uboot_env.c
>> > > > +++ b/src/uboot_env.c
>> > > > @@ -20,6 +20,7 @@
>> > > > #include <stddef.h>
>> > > > #include <dirent.h>
>> > > > #include <unistd.h>
>> > > > +#include <limits.h>
>> > > > #include <linux/fs.h>
>> > > > #include <string.h>
>> > > > #include <fcntl.h>
>> > > > @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct
>> uboot_ctx
>> > > *ctx, const char *config)
>> > > > int ndev = 0;
>> > > > struct uboot_flash_env *dev;
>> > > > char *tmp;
>> > > > + char *path;
>> > > > int retval = 0;
>> > > >
>> > > > if (!config)
>> > > > @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct
>> uboot_ctx
>> > > *ctx, const char *config)
>> > > > ctx->size = dev->envsize;
>> > > >
>> > > > if (tmp) {
>> > > > + if ((path = realpath(tmp, NULL)) ==
>> > NULL) {
>> > > > + free(tmp);
>> > > > + retval = -EINVAL;
>> > > > + break;
>> > > > + }
>> > > > memset(dev->devname, 0,
>> > sizeof(dev->devname));
>> > > > - strncpy(dev->devname, tmp,
>> > > sizeof(dev->devname) - 1);
>> > > > + strncpy(dev->devname, path,
>> > > sizeof(dev->devname) - 1);
>> > > > free(tmp);
>> > > > + free(path);
>> > > > }
>> > > >
>> > > > if (check_env_device(ctx, dev) < 0) {
>> > > >
>> > >
>> > > Looks good.
>> > >
>> > > Reviewed-by: Stefano Babic <sbabic@denx.de
>> > <mailto:sbabic@denx.de> <mailto:sbabic@denx.de <mailto:
>> sbabic@denx.de>>>
>> > >
>> > > Best regards,
>> > > Stefano Babic
>> > >
>> > > --
>> > >
>> >
>> =====================================================================
>> > > DENX Software Engineering GmbH, Managing Director:
>> > Wolfgang Denk
>> > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
>> > Germany
>> > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email:
>> > > sbabic@denx.de <mailto:sbabic@denx.de> <mailto:sbabic@denx.de
>> > <mailto:sbabic@denx.de>>
>> > >
>> >
>> =====================================================================
>> > >
>> >
>> >
>> > --
>> >
>> =====================================================================
>> > DENX Software Engineering GmbH, Managing Director: Wolfgang
>> Denk
>> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
>> Germany
>> > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email:
>> > sbabic@denx.de <mailto:sbabic@denx.de>
>> >
>> =====================================================================
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups "swupdate" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an email to swupdate+unsubscribe@googlegroups.com
>> > <mailto:swupdate+unsubscribe@googlegroups.com>.
>> > To view this discussion on the web visit
>> >
>> https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com
>> > <
>> https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com?utm_medium=email&utm_source=footer
>> >.
>>
>>
>> --
>> =====================================================================
>> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
>> =====================================================================
>>
>
Hi Ming, On 08.08.20 17:02, Ming Liu wrote: > Hi, Stefano: > > I think the issue I mentioned has been fixed by the following commit: > > ``` > Fix bug introduced by commit 52a70114 > > The return value from lseek is not checked correctly, causing that the > environment is not read or written if an offset is set. > > Signed-off-by: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> > ``` Ok > > will send a V2 of this patch. > Ok, thanks. Best regards, Stefano Babic > //Ming Liu > > Ming Liu <liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>> 於 2020年8 > 月3日 週一 下午12:43寫道： > > Sure, will do git bisect. > > //Ming Liu > > Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de>> 於 2020年8月3 > 日 週一 下午12:25寫道： > > Hi Ming, > > On 03.08.20 12:18, Ming Liu wrote: > > I have done more tests, it's sort of strange, the problem I > mentioned is > > not caused by my patch, but the current libubootenv on: > > ``` > > commit 4a0a4662e25471e8fcaf8767a4fa4f454c88a2e2 > > Merge: 6117831 41b5188 > > Author: Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de> > <mailto:sbabic@denx.de <mailto:sbabic@denx.de>>> > > Date: Thu Jul 2 14:37:39 2020 +0200 > > > > Merge pull request #7 from TomzBench/cmake-fix > > > > fixed install for static target, fixed BUILD_DOC > acknowledgement > > ``` > > > > does not work on my imx7 machine, with the following > fw_env.config: > > ``` > > # Block device name Device offset Env. size > > /dev/mmcblk0boot0 -0x2200 0x2000 > > ``` > > > > but the commit > > ``` > > commit 86bd30a14e153a18f670b25708795253d8736f0f > > Author: Kristian Amlie <kristian.amlie@northern.tech> > > Date: Mon Jun 22 14:06:24 2020 +0200 > > > > Restore ability to feed script file via stdin, using `-s -`. > > > > This used to work in the fw-utils based on U-Boot itself. > > > > Signed-off-by: Kristian Amlie <kristian.amlie@northern.tech> > > Reviewed-by: Stefano Babic <sbabic@denx.de > <mailto:sbabic@denx.de> <mailto:sbabic@denx.de > <mailto:sbabic@denx.de>>> > > ``` > > does work, this is the srcrev being used in OE, maybe we are > having a > > regression in the latest libubootenv? > > Please run git-bisect to find out which commit is the cause. > > Best regards, > Stefano Babic > > > > > > //Ming Liu > > > > Stefano Babic <sbabic@denx.de <mailto:sbabic@denx.de> > <mailto:sbabic@denx.de <mailto:sbabic@denx.de>>> 於 2020年7月30日 > > 週四 下午12:52寫道： > > > > On 30.07.20 12:51, Ming Liu wrote: > > > Thanks for the review, please hold on merging, I seems > found a issue > > > with this patch, will need more test and probably will > send a V2. > > > > > > > ok, thanks. > > > > Regards, > > Stefano > > > > > //Ming Liu > > > > > > > > > Hi Ming, > > > > > > On 29.07.20 21:39, liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>> > > > <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com>>> wrote: > > > > From: Ming Liu <liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> > > <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com>> <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>>>> > > > > > > > > Some platform uses softlinks to the devices that hold > > environment > > > > data. The mechanism used to read device type from > config is > > not robust > > > > in this case. Calculating the canonicalized absolute > > pathname of the > > > > device could fix the problem. > > > > > > > > Signed-off-by: Mathias Thore > <mathias.thore@atlascopco.com <mailto:mathias.thore@atlascopco.com> > > <mailto:mathias.thore@atlascopco.com > <mailto:mathias.thore@atlascopco.com>> > > > <mailto:mathias.thore@atlascopco.com > <mailto:mathias.thore@atlascopco.com> > > <mailto:mathias.thore@atlascopco.com > <mailto:mathias.thore@atlascopco.com>>>> > > > > Signed-off-by: Ming Liu <liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> > > <mailto:liu.ming50@gmail.com <mailto:liu.ming50@gmail.com>> > > > <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com> <mailto:liu.ming50@gmail.com > <mailto:liu.ming50@gmail.com>>>> > > > > --- > > > > src/uboot_env.c | 10 +++++++++- > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/uboot_env.c b/src/uboot_env.c > > > > index 25de4fb..aceaa98 100644 > > > > --- a/src/uboot_env.c > > > > +++ b/src/uboot_env.c > > > > @@ -20,6 +20,7 @@ > > > > #include <stddef.h> > > > > #include <dirent.h> > > > > #include <unistd.h> > > > > +#include <limits.h> > > > > #include <linux/fs.h> > > > > #include <string.h> > > > > #include <fcntl.h> > > > > @@ -1110,6 +1111,7 @@ int > libuboot_read_config(struct uboot_ctx > > > *ctx, const char *config) > > > > int ndev = 0; > > > > struct uboot_flash_env *dev; > > > > char *tmp; > > > > + char *path; > > > > int retval = 0; > > > > > > > > if (!config) > > > > @@ -1152,9 +1154,15 @@ int > libuboot_read_config(struct uboot_ctx > > > *ctx, const char *config) > > > > ctx->size = dev->envsize; > > > > > > > > if (tmp) { > > > > + if ((path = realpath(tmp, > NULL)) == > > NULL) { > > > > + free(tmp); > > > > + retval = -EINVAL; > > > > + break; > > > > + } > > > > memset(dev->devname, 0, > > sizeof(dev->devname)); > > > > - strncpy(dev->devname, tmp, > > > sizeof(dev->devname) - 1); > > > > + strncpy(dev->devname, path, > > > sizeof(dev->devname) - 1); > > > > free(tmp); > > > > + free(path); > > > > } > > > > > > > > if (check_env_device(ctx, dev) < 0) { > > > > > > > > > > Looks good. > > > > > > Reviewed-by: Stefano Babic <sbabic@denx.de > <mailto:sbabic@denx.de> > > <mailto:sbabic@denx.de <mailto:sbabic@denx.de>> > <mailto:sbabic@denx.de <mailto:sbabic@denx.de> > <mailto:sbabic@denx.de <mailto:sbabic@denx.de>>>> > > > > > > Best regards, > > > Stefano Babic > > > > > > -- > > > > > > ===================================================================== > > > DENX Software Engineering GmbH, Managing Director: > > Wolfgang Denk > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 > Groebenzell, > > Germany > > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > > > sbabic@denx.de <mailto:sbabic@denx.de> > <mailto:sbabic@denx.de <mailto:sbabic@denx.de>> > <mailto:sbabic@denx.de <mailto:sbabic@denx.de> > > <mailto:sbabic@denx.de <mailto:sbabic@denx.de>>> > > > > > > ===================================================================== > > > > > > > > > -- > > > ===================================================================== > > DENX Software Engineering GmbH, Managing Director: > Wolfgang Denk > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 > Groebenzell, Germany > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > > sbabic@denx.de <mailto:sbabic@denx.de> <mailto:sbabic@denx.de > <mailto:sbabic@denx.de>> > > > ===================================================================== > > > > -- > > You received this message because you are subscribed to the > Google > > Groups "swupdate" group. > > To unsubscribe from this group and stop receiving emails from > it, send > > an email to swupdate+unsubscribe@googlegroups.com > <mailto:swupdate%2Bunsubscribe@googlegroups.com> > > <mailto:swupdate+unsubscribe@googlegroups.com > <mailto:swupdate%2Bunsubscribe@googlegroups.com>>. > > To view this discussion on the web visit > > > https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com > > > <https://groups.google.com/d/msgid/swupdate/CALatG%3D42C5f3noV%3D8fOwnpkSf-Pp3cXkvV5SAG03eZUzkbFt6w%40mail.gmail.com?utm_medium=email&utm_source=footer>. > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang > Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > sbabic@denx.de <mailto:sbabic@denx.de> > ===================================================================== >
diff --git a/src/uboot_env.c b/src/uboot_env.c index 25de4fb..aceaa98 100644 --- a/src/uboot_env.c +++ b/src/uboot_env.c @@ -20,6 +20,7 @@ #include <stddef.h> #include <dirent.h> #include <unistd.h> +#include <limits.h> #include <linux/fs.h> #include <string.h> #include <fcntl.h> @@ -1110,6 +1111,7 @@ int libuboot_read_config(struct uboot_ctx *ctx, const char *config) int ndev = 0; struct uboot_flash_env *dev; char *tmp; + char *path; int retval = 0; if (!config) @@ -1152,9 +1154,15 @@ int libuboot_read_config(struct uboot_ctx *ctx, const char *config) ctx->size = dev->envsize; if (tmp) { + if ((path = realpath(tmp, NULL)) == NULL) { + free(tmp); + retval = -EINVAL; + break; + } memset(dev->devname, 0, sizeof(dev->devname)); - strncpy(dev->devname, tmp, sizeof(dev->devname) - 1); + strncpy(dev->devname, path, sizeof(dev->devname) - 1); free(tmp); + free(path); } if (check_env_device(ctx, dev) < 0) {