Groups keyboard shortcuts have been updated
Dismiss
See shortcuts

[PATCH] BUG: util: Append missing slash to temporary mount dir

98 views
Skip to first unread message

Michael Glembotzki

unread,
Dec 19, 2024, 5:08:09 AM12/19/24
to swup...@googlegroups.com, Michael Glembotzki
The raw_handler requires a slash between the mountpoint and the file path.
Otherwise, no correct mount is performed, and the file is saved to the wrong
location.

[TRACE] : SWUPDATE running : [install_raw_file] : Installing file fitimage.itb.signed on /tmp/datadst/UEE6x2fitImage.signed

Fixes: 5d57a9c05ec2 (util: introduce generic mount helpers)
Fixes: aff67cdb2c62 (Make use of introduced swupdate_temporary_(u)mount)
Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>

---

The bug is pretty bad because, for example, it caused parts of the update
(e.g. for us the kernel fitimage) to not be installed. I could imagine that
some systems would not boot as expected.

Only the raw_handler has been tested. Other handlers, such as rdiff, btrfs,
archive, delta, could also be affected and should be explicitly tested.

core/util.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/core/util.c b/core/util.c
index 32104279..da3e815b 100644
--- a/core/util.c
+++ b/core/util.c
@@ -873,6 +873,7 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
char *mountpoint;
const char *dir;
int ret = 0;
+ unsigned int len;

if (type != MNT_SCRIPTS && type != MNT_DATA && type != MNT_BOOT_SCRIPTS)
return NULL;
@@ -883,10 +884,13 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
}

dir = mount_points[type];
- if (asprintf(&mountpoint, "%s%sXXXXXX", get_tmpdir(), dir) == -1) {
+ len = strlen(get_tmpdir()) + strlen(dir) + 8; /* 6 times X, / and \0 */
+ mountpoint = (char*) calloc(len, sizeof(char));
+ if (!mountpoint) {
ERROR("Unable to allocate memory");
return NULL;
}
+ snprintf(mountpoint, len, "%s%sXXXXXX", get_tmpdir(), dir);

if (!mkdtemp(mountpoint)) {
TRACE("Unable to create a unique temporary directory %s: %s",
@@ -904,6 +908,8 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
return NULL;
}

+ mountpoint[len-2] = '/';
+
return mountpoint;
}

--
2.44.1

Einar Jon Gunnarsson

unread,
Dec 19, 2024, 5:46:20 AM12/19/24
to swupdate
Hello Michael

Is there any reason why this isn't just

diff --git a/core/util.c b/core/util.c
index 32104279..c210a3e1 100644

--- a/core/util.c
+++ b/core/util.c
@@ -873,6 +873,7 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
        char *mountpoint;
        const char *dir;
        int ret = 0;
+       unsigned int len;
 
        if (type != MNT_SCRIPTS && type != MNT_DATA && type != MNT_BOOT_SCRIPTS)
                return NULL;
@@ -883,10 +884,12 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const

        }
 
        dir = mount_points[type];
-       if (asprintf(&mountpoint, "%s%sXXXXXX", get_tmpdir(), dir) == -1) {
+       len = asprintf(&mountpoint, "%s%sXXXXXX/", get_tmpdir(), dir);
+       if (len == -1) {

                ERROR("Unable to allocate memory");
                return NULL;
        }
+       mountpoint[len-1] = '\0'; /* last six characters of template must be XXXXXX */

 
        if (!mkdtemp(mountpoint)) {
                TRACE("Unable to create a unique temporary directory %s: %s",
@@ -904,6 +907,7 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
                return NULL;
        }
 
+       mountpoint[len-1] = '/';  /* restore the trailing slash */
        return mountpoint;
 }
 
Best regards
Einar Jon

Michael Glembotzki

unread,
Dec 19, 2024, 7:24:30 AM12/19/24
to swup...@googlegroups.com, Michael Glembotzki
The raw_handler requires a slash between the mountpoint and the file path.
Otherwise, no correct mount is performed, and the file is saved to the wrong
location.

[TRACE] : SWUPDATE running : [install_raw_file] : Installing file fitimage.itb.signed on /tmp/datadst/UEE6x2fitImage.signed

Fixes: 5d57a9c05ec2 (util: introduce generic mount helpers)
Fixes: aff67cdb2c62 (Make use of introduced swupdate_temporary_(u)mount)
Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

The bug is pretty bad because, for example, it caused parts of the update
(e.g. for us the kernel fitimage) to not be installed. I could imagine that
some systems would not boot as expected.

Only the raw_handler has been tested. Other handlers, such as rdiff, btrfs,
archive and delta should be explicitly tested.

Changes since V1:
- Simplify Patch according to Einar Jon Gunnarsson suggestion



core/util.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/core/util.c b/core/util.c
index 32104279..a4973b29 100644
--- a/core/util.c
+++ b/core/util.c
@@ -873,6 +873,7 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
char *mountpoint;
const char *dir;
int ret = 0;
+ unsigned int len;

if (type != MNT_SCRIPTS && type != MNT_DATA && type != MNT_BOOT_SCRIPTS)
return NULL;
@@ -883,11 +884,14 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
}

dir = mount_points[type];
- if (asprintf(&mountpoint, "%s%sXXXXXX", get_tmpdir(), dir) == -1) {
+ len = asprintf(&mountpoint, "%s%sXXXXXX/", get_tmpdir(), dir);
+ if (len == -1) {
ERROR("Unable to allocate memory");
return NULL;
}

+ mountpoint[len-1] = '\0'; /* last six characters of template must be XXXXXX */
+
if (!mkdtemp(mountpoint)) {
TRACE("Unable to create a unique temporary directory %s: %s",
mountpoint, strerror(errno));
@@ -904,6 +908,8 @@ char *swupdate_temporary_mount(tmp_mountpoint_t type, const char *device, const
return NULL;
}

+ mountpoint[len-1] = '/'; /* restore the trailing slash */

Michael Glembotzki

unread,
Jan 9, 2025, 9:38:39 AMJan 9
to swupdate
Hi,

I think the topic is urgend. Any concerns mergen the Patch? We might need a new release IMO.

Best regards,
Michael

Daniel Braunwarth

unread,
Jan 13, 2025, 5:00:27 AMJan 13
to swup...@googlegroups.com, Michael Glembotzki
Hi Michael
> --
> 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+u...@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/swupdate/20241219121842.412062-2-Michael.Glembotzki%40iris-sensing.com.
>

Reviewed-by: Daniel Braunwarth <o...@braunwarth.dev>
Tested-by: Daniel Braunwarth <o...@braunwarth.dev>

Michael Glembotzki

unread,
Jan 22, 2025, 3:40:49 AMJan 22
to swupdate
Hello Stefano, hello Daniel,

I would like to point out again that the bug creates a regression. SWUpdate runs through / reports success without copying the file. Users of the raw_handler should not use the unpatched version 2024.12 in the field. Do we want to add the patch to meta-swupdate or create a new release?

Best regards,
Michael

Stefano Babic

unread,
Jan 22, 2025, 4:24:10 AMJan 22
to Michael Glembotzki, swupdate
Hi Michael,

On 1/22/25 09:40, Michael Glembotzki wrote:
> Hello Stefano, hello Daniel,
>
> I would like to point out again that the bug creates a regression.

Sure.

> SWUpdate runs through / reports success without copying the file. Users
> of the raw_handler should not use the unpatched version 2024.12 in the
> field. Do we want to add the patch to meta-swupdate or create a new release?
>

From OE point of view, a patch in meta-swupdate with a
swupdate_2024.12.bbappend solves it. But there is not just OE.

For Debian / ISAR, this remains unsolved and it is much better to have a
fix tag. So rather, I admit the better solution is a release. And as
there are already 35 patches on top of 2024.12 and they could create
further regression issues, the safest (but outside my normal workflow)
way is to create a branch 2024.12-y and tag it with just this patch.

Best regards,
Stefano Babic

> Best regards,
> Michael
>
> Daniel Braunwarth schrieb am Montag, 13. Januar 2025 um 11:00:27 UTC+1:
>
> Hi Michael
>
> December 19, 2024 at 1:18 PM, "Michael Glembotzki"
> <m.gl...@gmail.com mailto:m.gl...@gmail.com?
> to=%22Michael%20Glembotzki%22%20%3Cm.glembo%40gmail.com
> sensing.com <https://groups.google.com/d/msgid/
> swupdate/20241219121842.412062-2-Michael.Glembotzki%40iris-
> sensing.com>.
> >
>
> Reviewed-by: Daniel Braunwarth <o...@braunwarth.dev>
> Tested-by: Daniel Braunwarth <o...@braunwarth.dev>
>
> --
> 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+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/f04f747e-593e-470f-9276-ddf5d63a1223n%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/f04f747e-593e-470f-9276-
> ddf5d63a1223n%40googlegroups.com?utm_medium=email&utm_source=footer>.

Michael Glembotzki

unread,
Jan 22, 2025, 4:47:19 AMJan 22
to swupdate
Hi Stefano,

Stefano Babic schrieb am Mittwoch, 22. Januar 2025 um 10:24:10 UTC+1:
Hi Michael,

On 1/22/25 09:40, Michael Glembotzki wrote:
> Hello Stefano, hello Daniel,
>
> I would like to point out again that the bug creates a regression.

Sure.

> SWUpdate runs through / reports success without copying the file. Users
> of the raw_handler should not use the unpatched version 2024.12 in the
> field. Do we want to add the patch to meta-swupdate or create a new release?
>

From OE point of view, a patch in meta-swupdate with a
swupdate_2024.12.bbappend solves it. But there is not just OE.

For Debian / ISAR, this remains unsolved and it is much better to have a
fix tag. So rather, I admit the better solution is a release. And as
there are already 35 patches on top of 2024.12 and they could create
further regression issues, the safest (but outside my normal workflow)
way is to create a branch 2024.12-y and tag it with just this patch.

Agreed. If I can help in any way, let me know.

Could we add a testing phase 14 days before the next regular release?

Best regards,
Michael
 
Best regards,
Stefano Babic

Michael Adler

unread,
Jan 22, 2025, 5:00:30 AMJan 22
to Michael Glembotzki, swupdate
Hi all,

> Agreed. If I can help in any way, let me know.
>
> Could we add a testing phase 14 days before the next regular release?

Just to share my thoughts - considering the severity of the bug and given our
new GitHub CI setup, I believe it would be useful to have an automated test
case for this. Perhaps not a unit test, but rather a "real world" test case.

Kind Regards,
Michael

--
Michael Adler

Siemens AG
Technology
Connectivity & Edge
Open Source Embedded Systems
FT RPD CED OES-DE
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany

Siemens Aktiengesellschaft: Chairman of the Supervisory Board: Jim Hagemann
Snabe; Managing Board: Roland Busch, Chairman, President and Chief Executive
Officer; Cedrik Neike, Matthias Rebellius, Ralf P. Thomas, Judith Wiese;
Registered offices: Berlin and Munich, Germany; Commercial registries:
Berlin-Charlottenburg, HRB 12300, Munich, HRB 6684; WEEE-Reg.-No. DE 23691322

Stefano Babic

unread,
Jan 22, 2025, 7:19:24 AMJan 22
to Michael Adler, Michael Glembotzki, swupdate
Hi Everybody,

On 22.01.25 11:00, 'Michael Adler' via swupdate wrote:
> Hi all,
>
>> Agreed. If I can help in any way, let me know.
>>
>> Could we add a testing phase 14 days before the next regular release?
>
> Just to share my thoughts - considering the severity of the bug and given our
> new GitHub CI setup, I believe it would be useful to have an automated test
> case for this. Perhaps not a unit test, but rather a "real world" test case.
>

This is not unknown and I have already raised the issue. It is
documented in the improvements section, see:

https://github.com/sbabic/swupdate/blob/master/doc/source/improvement_proposals.rst#test-and-continuous-integration

In the past, I took time to test on the projects that I had at the time
of the release. Not everything can be checked. But I was mostly the only
contributor. With increasing number of patches, that is of course a good
thing for the project, this does not match anymore. In last two release
I mostly relied on external testers, and patches were merged on master
with low test from my site, and with the hope that something is reported
before the official release. This does not work, too, because everybody
is working on projects with timelines, etc., and the next SWUpdate
release is not urgent. 2023.12 needed to be patched, 2024.05 twice,
2024.12 is needed, too.

The github integration, recently introduced, is more for users able to
push on their branches / users. As stated before, Github is a
proprietary platform and they provide a max amount of free minutes for
the runners. And Github is of course right, I am using their resources.
Nothing to say against. I am not sure how minutes are counted, as after
just pushing few commits, 84 minutes were already consumed. So this
probably does not scale by increasing tests etc., but there are other
reasons.

What I would like to have is a small factory, with some real hardware
where new release is actively tested. And via gitlab-ci and test
frameworks, everything could be automatized. Bandwidth can be optimiyed
as factory and server resides on the server room, and more tests can be
executed. What I think it is good, is to have enough hardware to test at
least these configurations:

Architectures:
- Intel x86
- ARM
- ARM64

Boot:
- U-Boot (of course)
- Grub
- efiBootGuard

Storage:
- SSD / disks
- Raw NAND + UBI
- Raw NOR Flash

And both single-copy and A/B should be automatically tested. As reported
in my previous answer to Mark, pipelines are currently not accessible
outside. I have no problem to make them public to see results, as I
always run them before pushing to the official repo. But I do not want
to make possible that everyone can push and start runners, as this is
resource hungry.

So this CI with real hardware requires some investition, and I can state
here that I have already tried this year with one of public fund
subsidized by Germany in case of FOSS projects with public utility (I
think SWUpdate is). Answer will be later in second quarter 2025. If this
happens, I will have resources to implement it.

I should maybe raise the priority of this item from medium to high in
the improvements file, so that I make clearer to more users its importance.

Best regards,
Stefano


ayoub...@googlemail.com

unread,
Jan 22, 2025, 7:59:34 AMJan 22
to swupdate
Hallo Stefano,

wouldn't make sens to have first a minimal test Setup without real HW using Qemu for example ? Nand can be simulated within Qemu.

best regards

Daniel Braunwarth

unread,
Jan 22, 2025, 8:23:43 AMJan 22
to Stefano Babic, Michael Adler, Michael Glembotzki, swupdate
Hello everyone


January 22, 2025 at 1:19 PM, "Stefano Babic" <stefan...@swupdate.org mailto:stefan...@swupdate.org?to=%22Stefano%20Babic%22%20%3Cstefano.babic%40swupdate.org%3E > wrote:
>
> Hi Everybody,
>
> On 22.01.25 11:00, 'Michael Adler' via swupdate wrote:
>
> >
> > Hi all,
> >
> > >
> > > Agreed. If I can help in any way, let me know.
> > >
> > > Could we add a testing phase 14 days before the next regular release?
> > >
> > Just to share my thoughts - considering the severity of the bug and given our
> > new GitHub CI setup, I believe it would be useful to have an automated test
> > case for this. Perhaps not a unit test, but rather a "real world" test case.
> >
> This is not unknown and I have already raised the issue. It is
> documented in the improvements section, see:
>
> https://github.com/sbabic/swupdate/blob/master/doc/source/improvement_proposals.rst#test-and-continuous-integration

I could imagine working on this. But first I would like to finish my work on meson. ;)

On the other side I'm already fighting with the Makefile based acceptance tests.
So maybe I'll dig into this topic.

> In the past, I took time to test on the projects that I had at the time
> of the release. Not everything can be checked. But I was mostly the only
> contributor. With increasing number of patches, that is of course a good
> thing for the project, this does not match anymore. In last two release
> I mostly relied on external testers, and patches were merged on master
> with low test from my site, and with the hope that something is reported
> before the official release. This does not work, too, because everybody
> is working on projects with timelines, etc., and the next SWUpdate
> release is not urgent. 2023.12 needed to be patched, 2024.05 twice,
> 2024.12 is needed, too.
>
> The github integration, recently introduced, is more for users able to
> push on their branches / users. As stated before, Github is a
> proprietary platform and they provide a max amount of free minutes for
> the runners. And Github is of course right, I am using their resources.
> Nothing to say against. I am not sure how minutes are counted, as after
> just pushing few commits, 84 minutes were already consumed. So this
> probably does not scale by increasing tests etc., but there are other
> reasons.

Where is this limitation for the free GitHub runners coming from?

As far as I can see in the official GitHub documentation there is no
monthly runtime limit for the standard runners.

https://docs.github.com/en/actions/administering-github-actions/usage-limits-billing-and-administration

>
> What I would like to have is a small factory, with some real hardware
> where new release is actively tested. And via gitlab-ci and test
> frameworks, everything could be automatized. Bandwidth can be optimiyed
> as factory and server resides on the server room, and more tests can be
> executed. What I think it is good, is to have enough hardware to test at
> least these configurations:
>
> Architectures:
> - Intel x86
> - ARM
> - ARM64
>
> Boot:
> - U-Boot (of course)
> - Grub
> - efiBootGuard
>
> Storage:
> - SSD / disks
> - Raw NAND + UBI
> - Raw NOR Flash

Speaking for my employer I can say - if we find a way to upstream all the changes
we are maintaining locally (which is mostly the rework of reboot/postinst handling), we
could try to integrate SWUpdate on a daily basis into our product tests.

This would cover arm64, u-boot, eMMC, single-copy and A/B updates.

Another thing we would need to solve is the integration into a Debian package we
could consume. But I think this is something we can solve.

Regards
Daniel

Stefano Babic

unread,
Jan 22, 2025, 8:57:27 AMJan 22
to ayoub...@googlemail.com, swupdate
On 1/22/25 13:59, 'ayoub...@googlemail.com' via swupdate wrote:
> Hallo Stefano,
>
> wouldn't make sens to have first a minimal test Setup without real HW
> using Qemu for example ? Nand can be simulated within Qemu.

We can use nandsim, but most issues with NAND are related to real hardware.

>
> best regards
>
> On Wednesday, January 22, 2025 at 1:19:24 PM UTC+1 Stefano Babic wrote:
>
> Hi Everybody,
>
> On 22.01.25 11:00, 'Michael Adler' via swupdate wrote:
> > Hi all,
> >
> >> Agreed. If I can help in any way, let me know.
> >>
> >> Could we add a testing phase 14 days before the next regular
> release?
> >
> > Just to share my thoughts - considering the severity of the bug
> and given our
> > new GitHub CI setup, I believe it would be useful to have an
> automated test
> > case for this. Perhaps not a unit test, but rather a "real world"
> test case.
> >
>
> This is not unknown and I have already raised the issue. It is
> documented in the improvements section, see:
>
> https://github.com/sbabic/swupdate/blob/master/doc/source/
> improvement_proposals.rst#test-and-continuous-integration <https://
> github.com/sbabic/swupdate/blob/master/doc/source/
> improvement_proposals.rst#test-and-continuous-integration>
> --
> 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+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/d19a643d-14d3-49a3-8ae9-5b6d9a089760n%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/
> d19a643d-14d3-49a3-8ae9-5b6d9a089760n%40googlegroups.com?
> utm_medium=email&utm_source=footer>.

Stefano Babic

unread,
Jan 22, 2025, 9:02:32 AMJan 22
to Daniel Braunwarth, Stefano Babic, Michael Adler, Michael Glembotzki, swupdate
Hi Daniel,
https://docs.github.com/en/billing/managing-billing-for-your-products/managing-billing-for-github-actions/about-billing-for-github-actions

> As far as I can see in the official GitHub documentation there is no
> monthly runtime limit for the standard runners.
>
> https://docs.github.com/en/actions/administering-github-actions/usage-limits-billing-and-administration

These are the limitations, for example max concurrent jobs.

But as far as I won't be billed, there is no reason I stop the feature.

Regards,
Stefano
Reply all
Reply to author
Forward
0 new messages