[PATCH] image: check if the file is core dump

[PATCH] image: check if the file is core dump

[Zhibin Dong]

The previous code does a wrong judgement in two cases:
1. a file is suffixed by .core but is not a core dump file
2. a file is a core dump file but is not suffixed by .core

The new code uses `file` to determine the type of files, which is more
accurate.

Signed-off-by: Zhibin Dong <zhibin.dong@siemens.com>
---
 meta/classes/image.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 98741da0..10923947 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -444,7 +444,7 @@ EOSUDO
 
     # Sometimes qemu-user-static generates coredumps in chroot, move them
     # to work temporary directory and inform user about it.
-    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
+    for f in $(sudo find ${ROOTFSDIR} -type f -exec file {} \; | grep 'core file' | cut -d: -f1); do
         sudo mv "${f}" "${WORKDIR}/temp/"
         bbwarn "found core dump in rootfs, check it in ${WORKDIR}/temp/${f##*/}"
     done
-- 
2.39.2

Re: [PATCH] image: check if the file is core dump

[MOESSBAUER, Felix]

On Tue, 2024-04-16 at 13:12 +0800, Zhibin Dong wrote:
> The previous code does a wrong judgement in two cases:
> 1. a file is suffixed by .core but is not a core dump file
> 2. a file is a core dump file but is not suffixed by .core

Hi, just for reference: This is an addition to the pattern implemented
in https://groups.google.com/g/isar-users/c/JtCExK7m4OY/m/F9QaxKBSAQAJ

> 
> The new code uses `file` to determine the type of files, which is
> more
> accurate.
> 
> Signed-off-by: Zhibin Dong <zhibin.dong@siemens.com>
> ---
>  meta/classes/image.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 98741da0..10923947 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -444,7 +444,7 @@ EOSUDO
>  
>      # Sometimes qemu-user-static generates coredumps in chroot, move
> them
>      # to work temporary directory and inform user about it.
> -    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> +    for f in $(sudo find ${ROOTFSDIR} -type f -exec file {} \; |

Are we sure this works in cross-arch scenarios? I.e. can an amd64
"file" detect arm64 coredumps? Did you test this Zhibin Dong?
If so, this patch is fine.

Best regards,
Felix

> grep 'core file' | cut -d: -f1); do
>          sudo mv "${f}" "${WORKDIR}/temp/"
>          bbwarn "found core dump in rootfs, check it in
> ${WORKDIR}/temp/${f##*/}"
>      done
> -- 
> 2.39.2
> 

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH] image: check if the file is core dump

[Zhibin Dong]

[-- Attachment #1.1: Type: text/plain, Size: 2603 bytes --]

 

> Are we sure this works in cross-arch scenarios? I.e. can an amd64 "file" 
detect arm64 coredumps? Did you test this Zhibin Dong?
> If so, this patch is fine. 

On an amd64 platform, I execute these commands.
$ file ./core.amd64
./core.amd64: ELF 64-bit LSB core file, x86-64, version 1 (SYSV), 
SVR4-style, from './crashing_program', real uid: 1000, effective uid: 1000, 
real gid: 1000, effective gid: 1000, execfn: './crashing_program', 
platform: 'x86_64'
$ file ./core.aarch64
./core.aarch64: ELF 64-bit LSB core file, ARM aarch64, version 1 (SYSV), 
SVR4-style, from './crashing_program_aarch64', real uid: 1000, effective 
uid: 1000, real gid: 1001, effective gid: 1001, execfn: 
'./crashing_program_aarch64', platform: 'aarch64'

`file` can determine the architecture of a file and also determine if it is 
a core file.
在2024年4月17日星期三 UTC+8 05:02:35<MOESSBAUER, Felix> 写道：

> On Tue, 2024-04-16 at 13:12 +0800, Zhibin Dong wrote:
> > The previous code does a wrong judgement in two cases:
> > 1. a file is suffixed by .core but is not a core dump file
> > 2. a file is a core dump file but is not suffixed by .core
>
> Hi, just for reference: This is an addition to the pattern implemented
> in https://groups.google.com/g/isar-users/c/JtCExK7m4OY/m/F9QaxKBSAQAJ
>
> > 
> > The new code uses `file` to determine the type of files, which is
> > more
> > accurate.
> > 
> > Signed-off-by: Zhibin Dong <zhibi...@siemens.com>
> > ---
> >  meta/classes/image.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index 98741da0..10923947 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -444,7 +444,7 @@ EOSUDO
> >  
> >      # Sometimes qemu-user-static generates coredumps in chroot, move
> > them
> >      # to work temporary directory and inform user about it.
> > -    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> > +    for f in $(sudo find ${ROOTFSDIR} -type f -exec file {} \; |
>
> Are we sure this works in cross-arch scenarios? I.e. can an amd64
> "file" detect arm64 coredumps? Did you test this Zhibin Dong?
> If so, this patch is fine.
>
> Best regards,
> Felix
>
> > grep 'core file' | cut -d: -f1); do
> >          sudo mv "${f}" "${WORKDIR}/temp/"
> >          bbwarn "found core dump in rootfs, check it in
> > ${WORKDIR}/temp/${f##*/}"
> >      done
> > -- 
> > 2.39.2
> > 
>
> -- 
> Siemens AG, Technology
> Linux Expert Center
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 4045 bytes --]

RE: [PATCH] image: check if the file is core dump

[Schmidt, Adriaan]

Zhibin Dong, Dienstag, 16. April 2024 07:12
> 
> The previous code does a wrong judgement in two cases:
> 1. a file is suffixed by .core but is not a core dump file
> 2. a file is a core dump file but is not suffixed by .core
> 
> The new code uses `file` to determine the type of files, which is more
> accurate.
> 
> Signed-off-by: Zhibin Dong <zhibin.dong@siemens.com>
> ---
>  meta/classes/image.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 98741da0..10923947 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -444,7 +444,7 @@ EOSUDO
> 
>      # Sometimes qemu-user-static generates coredumps in chroot, move them
>      # to work temporary directory and inform user about it.
> -    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> +    for f in $(sudo find ${ROOTFSDIR} -type f -exec file {} \; | grep 'core

This runs `file` on every file in the rootfs, which I expect can take a long time.

Also, if you check `file -l | grep core` you will see that not all file types
that have "core" in their description are actually core dumps.

I don't know the details of why we have this code [1], but maybe "scan the whole
rootfs" is not the best solution to the problem...

When specifically can those core dumps happen?
Is it only during update-initramfs, which is mentioned in the bug linked to the original commit [2]?
Maybe also during package installation, which may run commands with qemu-user?
Can we reproduce this?
Where in the rootfs are they stored?
Can our search for them be more targeted?
Would such core dumps be caught by other checks we have in place (e.g., modification time of files)?

Adriaan

[1] introduced in fa10b1d9b3a5e876bbcf556b03d585bf712fa7a5
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981

> file' | cut -d: -f1); do
>          sudo mv "${f}" "${WORKDIR}/temp/"
>          bbwarn "found core dump in rootfs, check it in
> ${WORKDIR}/temp/${f##*/}"
>      done
> --
> 2.39.2
> 
> --
> You received this message because you are subscribed to the Google Groups
> "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to isar-users+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/isar-users/20240416051222.3344127-1-
> zhibin.dong%40siemens.com.

[PATCH] image: check if the file is core dump

[Zhibin Dong]

The previous code does a wrong judgement in two cases:
1. a file is suffixed by .core but is not a core dump file
2. a file is a core dump file but is not suffixed by .core

The new code uses `file` to determine the type of files, which is more
accurate.

Signed-off-by: Zhibin Dong <zhibin.dong@siemens.com>
---
 meta/classes/image.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 98741da0..2b0995d2 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -444,7 +444,7 @@ EOSUDO
 
     # Sometimes qemu-user-static generates coredumps in chroot, move them
     # to work temporary directory and inform user about it.
-    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
+    for f in $(sudo find ${ROOTFSDIR} -type f -exec file --mime-type {} \; | grep 'application/x-coredump' | cut -d: -f1); do
         sudo mv "${f}" "${WORKDIR}/temp/"
         bbwarn "found core dump in rootfs, check it in ${WORKDIR}/temp/${f##*/}"
     done
-- 
2.39.2

Re: [PATCH] image: check if the file is core dump

[Zhibin Dong]

[-- Attachment #1.1: Type: text/plain, Size: 1210 bytes --]

How about using `--mime-type`?

在2024年4月17日星期三 UTC+8 14:47:25<Zhibin Dong> 写道：

> The previous code does a wrong judgement in two cases:
> 1. a file is suffixed by .core but is not a core dump file
> 2. a file is a core dump file but is not suffixed by .core
>
> The new code uses `file` to determine the type of files, which is more
> accurate.
>
> Signed-off-by: Zhibin Dong <zhibi...@siemens.com>
> ---
> meta/classes/image.bbclass | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 98741da0..2b0995d2 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -444,7 +444,7 @@ EOSUDO
>
> # Sometimes qemu-user-static generates coredumps in chroot, move them
> # to work temporary directory and inform user about it.
> - for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> + for f in $(sudo find ${ROOTFSDIR} -type f -exec file --mime-type {} \; | 
> grep 'application/x-coredump' | cut -d: -f1); do
> sudo mv "${f}" "${WORKDIR}/temp/"
> bbwarn "found core dump in rootfs, check it in ${WORKDIR}/temp/${f##*/}"
> done
> -- 
> 2.39.2
>
>

[-- Attachment #1.2: Type: text/html, Size: 1617 bytes --]

Re: [PATCH] image: check if the file is core dump

[Uladzimir Bely]

On Tue, 2024-04-16 at 23:50 -0700, Zhibin Dong wrote:
> How about using `--mime-type`?
> 
> 在2024年4月17日星期三 UTC+8 14:47:25<Zhibin Dong> 写道：
> > The previous code does a wrong judgement in two cases:
> > 1. a file is suffixed by .core but is not a core dump file
> > 2. a file is a core dump file but is not suffixed by .core
> > 
> > The new code uses `file` to determine the type of files, which is
> > more
> > accurate.
> > 
> > Signed-off-by: Zhibin Dong <zhibi...@siemens.com>
> > ---
> >  meta/classes/image.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/image.bbclass
> > b/meta/classes/image.bbclass
> > index 98741da0..2b0995d2 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -444,7 +444,7 @@ EOSUDO
> >  
> >  # Sometimes qemu-user-static generates coredumps in chroot, move
> > them
> >  # to work temporary directory and inform user about it.
> > - for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> > + for f in $(sudo find ${ROOTFSDIR} -type f -exec file --mime-type
> > {} \; | grep 'application/x-coredump' | cut -d: -f1); do
> >  sudo mv "${f}" "${WORKDIR}/temp/"
> >  bbwarn "found core dump in rootfs, check it in
> > ${WORKDIR}/temp/${f##*/}"
> >  done

Applied to next, thanks.

-- 
Best regards,
Uladzimir.

Re: [PATCH] image: check if the file is core dump

[Jan Kiszka]

On 17.04.24 08:46, Zhibin Dong wrote:
> The previous code does a wrong judgement in two cases:
> 1. a file is suffixed by .core but is not a core dump file
> 2. a file is a core dump file but is not suffixed by .core
> 
> The new code uses `file` to determine the type of files, which is more
> accurate.
> 
> Signed-off-by: Zhibin Dong <zhibin.dong@siemens.com>
> ---
>  meta/classes/image.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 98741da0..2b0995d2 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -444,7 +444,7 @@ EOSUDO
>  
>      # Sometimes qemu-user-static generates coredumps in chroot, move them
>      # to work temporary directory and inform user about it.
> -    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> +    for f in $(sudo find ${ROOTFSDIR} -type f -exec file --mime-type {} \; | grep 'application/x-coredump' | cut -d: -f1); do
>          sudo mv "${f}" "${WORKDIR}/temp/"
>          bbwarn "found core dump in rootfs, check it in ${WORKDIR}/temp/${f##*/}"
>      done

Unfortunately, this turns out to be extreeemly costly: For every file in
the rootfs, we now call 'file' which opens and reads its header to
determine whether it is a coredump. I suspect this was never really
tested against some non-trivial rootfs.

I agree that we would avoid false positives, thus should check for the
mime-type before deleting. But is there really a case for coredumps not
ending on .core?

Jan

-- 
Siemens AG, Technology
Linux Expert Center

RE: [PATCH] image: check if the file is core dump

[Schmidt, Adriaan]

Jan Kiszka, Donnerstag, 27. Juni 2024 17:44:
> On 17.04.24 08:46, Zhibin Dong wrote:
> > The previous code does a wrong judgement in two cases:
> > 1. a file is suffixed by .core but is not a core dump file
> > 2. a file is a core dump file but is not suffixed by .core
> >
> > The new code uses `file` to determine the type of files, which is more
> > accurate.
> >
> > Signed-off-by: Zhibin Dong <zhibin.dong@siemens.com>
> > ---
> >  meta/classes/image.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index 98741da0..2b0995d2 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -444,7 +444,7 @@ EOSUDO
> >
> >      # Sometimes qemu-user-static generates coredumps in chroot, move them
> >      # to work temporary directory and inform user about it.
> > -    for f in $(sudo find ${ROOTFSDIR} -type f -name *.core); do
> > +    for f in $(sudo find ${ROOTFSDIR} -type f -exec file --mime-type {} \;
> | grep 'application/x-coredump' | cut -d: -f1); do
> >          sudo mv "${f}" "${WORKDIR}/temp/"
> >          bbwarn "found core dump in rootfs, check it in
> ${WORKDIR}/temp/${f##*/}"
> >      done
> 
> Unfortunately, this turns out to be extreeemly costly: For every file in
> the rootfs, we now call 'file' which opens and reads its header to
> determine whether it is a coredump. I suspect this was never really
> tested against some non-trivial rootfs.
> 
> I agree that we would avoid false positives, thus should check for the
> mime-type before deleting. But is there really a case for coredumps not
> ending on .core?

Quoting my own reply [0] to the original patch:

> I don't know the details of why we have this code [1], but maybe "scan the whole
> rootfs" is not the best solution to the problem...
> When specifically can those core dumps happen?
> Is it only during update-initramfs, which is mentioned in the bug linked to the original commit [2]?
> Maybe also during package installation, which may run commands with qemu-user?
> Can we reproduce this?
> Where in the rootfs are they stored?
> Can our search for them be more targeted?
> Would such core dumps be caught by other checks we have in place (e.g., modification time of files)?

For now I fully agree with your proposal to only check files named *.core.

After that, if there is need for further optimization, I think it's worth examining
the questions above, and try to find a solution with a more targeted search (not scanning
the complete rootfs).

Adriaan

[0] https://groups.google.com/g/isar-users/c/w2KZ8IOyoF8/m/wGIug0kBAwAJ
[1] introduced in fa10b1d9b3a5e876bbcf556b03d585bf712fa7a5
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981

> Jan
> 
> --
> Siemens AG, Technology
> Linux Expert Center
> 
> --
> You received this message because you are subscribed to the Google Groups
> "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to isar-users+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/isar-users/9cd2657d-cc5f-47cd-8c9b-
> abd6091e7c43%40siemens.com.