diff mbox series

gptfdisk: add follow-up patch to fix with current popt

Message ID 20230119131853.27372-1-f_l_k@t-online.de
State Accepted, archived
Commit b2f3f8ced22da68eecd7689cc09e28f70100bd56
Headers show
Series gptfdisk: add follow-up patch to fix with current popt | expand

Commit Message

Markus Volk Jan. 19, 2023, 1:18 p.m. UTC
sgdisk still segfaults for some tasks (e.g 'sgdisk -v', 'sgdisk -V')

Add a follow-up patch that fixes the issue. It was taken from Archlinux

Signed-off-by: Markus Volk <f_l_k@t-online.de>
---
 .../fdisk/gptfdisk/popt-1.19-follow-up.patch  | 42 +++++++++++++++++++
 meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb |  1 +
 2 files changed, 43 insertions(+)
 create mode 100644 meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch

Comments

Alexander Kanavin Jan. 19, 2023, 3:14 p.m. UTC | #1
It’s not a backport as source comes from source forge. Please check other
patches to see the upstream situation and correct the status.

Alex

On Thu 19. Jan 2023 at 14.20, Markus Volk <f_l_k@t-online.de> wrote:

> sgdisk still segfaults for some tasks (e.g 'sgdisk -v', 'sgdisk -V')
>
> Add a follow-up patch that fixes the issue. It was taken from Archlinux
>
> Signed-off-by: Markus Volk <f_l_k@t-online.de>
> ---
>  .../fdisk/gptfdisk/popt-1.19-follow-up.patch  | 42 +++++++++++++++++++
>  meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb |  1 +
>  2 files changed, 43 insertions(+)
>  create mode 100644
> meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch
>
> diff --git
> a/meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch
> b/meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch
> new file mode 100644
> index 0000000000..a5ccd2385a
> --- /dev/null
> +++ b/meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch
> @@ -0,0 +1,42 @@
> +From f5de3401b974ce103ffd93af8f9d43505a04aaf9 Mon Sep 17 00:00:00 2001
> +From: Damian Kurek <starfire24680@gmail.com>
> +Date: Thu, 7 Jul 2022 03:39:16 +0000
> +Subject: [PATCH] Fix NULL dereference when duplicating string argument
> +
> +poptGetArg can return NULL if there are no additional arguments, which
> +makes strdup dereference NULL on strlen
> +
> +Upstream-Status: Backport [
> https://github.com/archlinux/svntogit-packages/tree/packages/gptfdisk/trunk
> ]
> +
> +Unsure if this patch is really a backport yet. It was taken from Archlinux
> +---
> + gptcl.cc | 6 ++++--
> + 1 file changed, 4 insertions(+), 2 deletions(-)
> +
> +diff --git a/gptcl.cc b/gptcl.cc
> +index 0d578eb..ab95239 100644
> +--- a/gptcl.cc
> ++++ b/gptcl.cc
> +@@ -155,10 +155,11 @@ int GPTDataCL::DoOptions(int argc, char* argv[]) {
> +    } // while
> +
> +    // Assume first non-option argument is the device filename....
> +-   device = strdup((char*) poptGetArg(poptCon));
> +-   poptResetContext(poptCon);
> ++   device = (char*) poptGetArg(poptCon);
> +
> +    if (device != NULL) {
> ++      device = strdup(device);
> ++      poptResetContext(poptCon);
> +       JustLooking(); // reset as necessary
> +       BeQuiet(); // Tell called functions to be less verbose &
> interactive
> +       if (LoadPartitions((string) device)) {
> +@@ -498,6 +499,7 @@ int GPTDataCL::DoOptions(int argc, char* argv[]) {
> +          cerr << "Error encountered; not saving changes.\n";
> +          retval = 4;
> +       } // if
> ++      free(device);
> +    } // if (device != NULL)
> +    poptFreeContext(poptCon);
> +    return retval;
> +
> diff --git a/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
> b/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
> index 66935b7fbb..cf0a60a1a9 100644
> --- a/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
> +++ b/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
> @@ -12,6 +12,7 @@ SRC_URI =
> "${SOURCEFORGE_MIRROR}/${BPN}/${PV}/${BP}.tar.gz \
>
> file://0001-Updated-guid.cc-to-deal-with-minor-change-in-libuuid.patch \
>
> file://0001-Fix-failure-crash-of-sgdisk-when-compiled-with-lates.patch \
>             file://0001-Use-64bit-time_t-on-linux-as-well.patch \
> +           file://popt-1.19-follow-up.patch \
>             "
>  SRC_URI[sha256sum] =
> "dafead2693faeb8e8b97832b23407f6ed5b3219bc1784f482dd855774e2d50c2"
>
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#176138):
> https://lists.openembedded.org/g/openembedded-core/message/176138
> Mute This Topic: https://lists.openembedded.org/mt/96376633/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Markus Volk Jan. 19, 2023, 3:16 p.m. UTC | #2
Am Do, 19. Jan 2023 um 16:14:58 +0100 schrieb Alexander Kanavin 
<alex.kanavin@gmail.com>:
> ck other patches to see the upstream situation and correct the status.

Then i guess the correct status would be pending ?
Alexander Kanavin Jan. 19, 2023, 3:17 p.m. UTC | #3
No. If it can be submitted upstream, then you should do so.

Alex

On Thu 19. Jan 2023 at 16.16, Markus Volk <f_l_k@t-online.de> wrote:

> Am Do, 19. Jan 2023 um 16:14:58 +0100 schrieb Alexander Kanavin <
> alex.kanavin@gmail.com>:
>
> ck other patches to see the upstream situation and correct the status.
>
>
> Then i guess the correct status would be pending ?
>
Markus Volk Jan. 19, 2023, 3:27 p.m. UTC | #4
Am Do, 19. Jan 2023 um 16:14:58 +0100 schrieb Alexander Kanavin 
<alex.kanavin@gmail.com>:
> ck other patches to see the upstream situation and correct the status.

But its neither my patch nor am I familiar with sourceforge or the 
actual issue with popt ... and I'm not keen on changing that. I think 
if the gptfdisk maintainer wants to release a new version, he will be 
able to find the patch on the net like I was.

And if he wants to get more help from the community it would be a good 
idea to have a git repo available somewhere. Or am I missing something 
and just wasn't able to find it?
Alexander Kanavin Jan. 19, 2023, 5:10 p.m. UTC | #5
On Thu, 19 Jan 2023 at 16:27, Markus Volk <f_l_k@t-online.de> wrote:
> But its neither my patch nor am I familiar with sourceforge or the actual issue with popt ... and I'm not keen on changing that. I think if the gptfdisk maintainer wants to release a new version, he will be able to find the patch on the net like I was.
>
> And if he wants to get more help from the community it would be a good idea to have a git repo available somewhere. Or am I missing something and just wasn't able to find it?

The git repo is in fact on sourceforge:
https://sourceforge.net/p/gptfdisk/code/ci/master/tree/
and you can open a merge request there. There's one from Khem for example.

Please do not come up with reasons to push this work (submitting
patches upstream) on me; I do not appreciate that.

Alex
Markus Volk Jan. 19, 2023, 5:22 p.m. UTC | #6
I don't want to push this work to you. Just wanted to give a reason, 
why I'd rather dont like to do myself, at least for the next days. Have 
still plenty of work lying here and in fact I do this only for fun and 
thats what it still needs to be to give me reason. I'm not related to 
technical things for a living. It's just, that I had an issue with 
gnome-disks and as a side effect I realized that sgdisk has an issue as 
well and found a fix for it.
But if thats not appropriate like this, just drop that commit for now. 
Will send a v2 when I've been digging into it enough, to have the 
feeling that I am halfway knowing what I talk about, if sending a merge 
request for a patch I didn't create.


Am Do, 19. Jan 2023 um 18:10:05 +0100 schrieb Alexander Kanavin 
<alex.kanavin@gmail.com>:
> On Thu, 19 Jan 2023 at 16:27, Markus Volk <f_l_k@t-online.de 
> <mailto:f_l_k@t-online.de>> wrote:
>>  But its neither my patch nor am I familiar with sourceforge or the 
>> actual issue with popt ... and I'm not keen on changing that. I 
>> think if the gptfdisk maintainer wants to release a new version, he 
>> will be able to find the patch on the net like I was.
>> 
>>  And if he wants to get more help from the community it would be a 
>> good idea to have a git repo available somewhere. Or am I missing 
>> something and just wasn't able to find it?
> 
> The git repo is in fact on sourceforge:
> <https://sourceforge.net/p/gptfdisk/code/ci/master/tree/>
> and you can open a merge request there. There's one from Khem for 
> example.
> 
> Please do not come up with reasons to push this work (submitting
> patches upstream) on me; I do not appreciate that.
> 
> Alex
Alexander Kanavin Jan. 19, 2023, 5:33 p.m. UTC | #7
I sent oe-core patches upstream that aren't mine all the time.
Sometimes I as well don't even understand them fully, and then I have
upstream explain to me what's wrong with it or why is it not
appropriate or poorly written. Or they rework it themselves. The
primary goal is to engage the upstream and make them aware, and no one
expects you to produce a perfect patch and complete understanding of
the issue in your head up-front.

My request is simply to go through the not-too-hard technicality of
sending the patch via opening a merge request, so that there is an
link for it that can be followed up later by you, or by someone else.
Later can be measured in weeks or months, and often is.

Alex

On Thu, 19 Jan 2023 at 18:22, Markus Volk <f_l_k@t-online.de> wrote:
>
> I don't want to push this work to you. Just wanted to give a reason, why I'd rather dont like to do myself, at least for the next days. Have still plenty of work lying here and in fact I do this only for fun and thats what it still needs to be to give me reason. I'm not related to technical things for a living. It's just, that I had an issue with gnome-disks and as a side effect I realized that sgdisk has an issue as well and found a fix for it.
> But if thats not appropriate like this, just drop that commit for now. Will send a v2 when I've been digging into it enough, to have the feeling that I am halfway knowing what I talk about, if sending a merge request for a patch I didn't create.
>
>
> Am Do, 19. Jan 2023 um 18:10:05 +0100 schrieb Alexander Kanavin <alex.kanavin@gmail.com>:
>
> On Thu, 19 Jan 2023 at 16:27, Markus Volk <f_l_k@t-online.de> wrote:
>
> But its neither my patch nor am I familiar with sourceforge or the actual issue with popt ... and I'm not keen on changing that. I think if the gptfdisk maintainer wants to release a new version, he will be able to find the patch on the net like I was. And if he wants to get more help from the community it would be a good idea to have a git repo available somewhere. Or am I missing something and just wasn't able to find it?
>
> The git repo is in fact on sourceforge: https://sourceforge.net/p/gptfdisk/code/ci/master/tree/ and you can open a merge request there. There's one from Khem for example. Please do not come up with reasons to push this work (submitting patches upstream) on me; I do not appreciate that. Alex
Markus Volk Jan. 19, 2023, 6:11 p.m. UTC | #8
Am Do, 19. Jan 2023 um 18:33:35 +0100 schrieb Alexander Kanavin 
<alex.kanavin@gmail.com>:
> no one
> expects you to produce a perfect patch and complete understanding of
> the issue in your head up-front.

I really didn't find the git repository and thought they were still 
using subversion. But I have to admit that I didn't search thoroughly. 
When I see code still hosted on Sourceforge, I'm automatically a bit 
skeptical.  It's not hard to do merge requests, but I really prefer to 
know something before I do it. And it's of course also a bit of a 
hassle.
The only thing I know for now is that it fixes a segfault, thats what I 
at least have tested. So it looks like an improvement.

But now that I have a git repo, I guess I lack the excuse ;)
Markus Volk Jan. 19, 2023, 7:20 p.m. UTC | #9
Am Do, 19. Jan 2023 um 16:14:58 +0100 schrieb Alexander Kanavin 
<alex.kanavin@gmail.com>:
> It’s not a backport as source comes from source forge.

After registering on sourceforge, I found out that the original author 
had submitted this patch about half a year ago without receiving any 
comment so far. This reinforces my prejudice against code that is still 
hosted there.

<https://sourceforge.net/p/gptfdisk/code/merge-requests/28/>
diff mbox series

Patch

diff --git a/meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch b/meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch
new file mode 100644
index 0000000000..a5ccd2385a
--- /dev/null
+++ b/meta/recipes-devtools/fdisk/gptfdisk/popt-1.19-follow-up.patch
@@ -0,0 +1,42 @@ 
+From f5de3401b974ce103ffd93af8f9d43505a04aaf9 Mon Sep 17 00:00:00 2001
+From: Damian Kurek <starfire24680@gmail.com>
+Date: Thu, 7 Jul 2022 03:39:16 +0000
+Subject: [PATCH] Fix NULL dereference when duplicating string argument
+
+poptGetArg can return NULL if there are no additional arguments, which
+makes strdup dereference NULL on strlen
+
+Upstream-Status: Backport [https://github.com/archlinux/svntogit-packages/tree/packages/gptfdisk/trunk]
+
+Unsure if this patch is really a backport yet. It was taken from Archlinux
+---
+ gptcl.cc | 6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/gptcl.cc b/gptcl.cc
+index 0d578eb..ab95239 100644
+--- a/gptcl.cc
++++ b/gptcl.cc
+@@ -155,10 +155,11 @@ int GPTDataCL::DoOptions(int argc, char* argv[]) {
+    } // while
+ 
+    // Assume first non-option argument is the device filename....
+-   device = strdup((char*) poptGetArg(poptCon));
+-   poptResetContext(poptCon);
++   device = (char*) poptGetArg(poptCon);
+ 
+    if (device != NULL) {
++      device = strdup(device);
++      poptResetContext(poptCon);
+       JustLooking(); // reset as necessary
+       BeQuiet(); // Tell called functions to be less verbose & interactive
+       if (LoadPartitions((string) device)) {
+@@ -498,6 +499,7 @@ int GPTDataCL::DoOptions(int argc, char* argv[]) {
+          cerr << "Error encountered; not saving changes.\n";
+          retval = 4;
+       } // if
++      free(device);
+    } // if (device != NULL)
+    poptFreeContext(poptCon);
+    return retval;
+
diff --git a/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb b/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
index 66935b7fbb..cf0a60a1a9 100644
--- a/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
+++ b/meta/recipes-devtools/fdisk/gptfdisk_1.0.9.bb
@@ -12,6 +12,7 @@  SRC_URI = "${SOURCEFORGE_MIRROR}/${BPN}/${PV}/${BP}.tar.gz \
            file://0001-Updated-guid.cc-to-deal-with-minor-change-in-libuuid.patch \
            file://0001-Fix-failure-crash-of-sgdisk-when-compiled-with-lates.patch \
            file://0001-Use-64bit-time_t-on-linux-as-well.patch \
+           file://popt-1.19-follow-up.patch \
            "
 SRC_URI[sha256sum] = "dafead2693faeb8e8b97832b23407f6ed5b3219bc1784f482dd855774e2d50c2"