Re: [PATCH v4 02/18] a.out: remove define-only CMAGIC, previously magic number

Re: [PATCH v4 02/18] a.out: remove define-only CMAGIC, previously magic number

[наб]

[-- Attachment #1: Type: text/plain, Size: 3858 bytes --]

Hi!

On Mon, Sep 26, 2022 at 10:16:02AM +0200, Geert Uytterhoeven wrote:
> Thanks for your patch, which is now commit 53c2bd679017277f
> ("a.out: remove define-only CMAGIC, previously magic number") in
> driver-core/driver-core-next.
> 
> On Fri, Sep 16, 2022 at 12:40 AM наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
> > The last user was removed in 5.1 in
> > commit 08300f4402ab ("a.out: remove core dumping support")
> > but this is part of the UAPI headers, so this may want to either wait
> > until a.out is removed entirely, or be removed from the magic number doc
> > and silently remain in the header
> 
> Indeed. This is part of uapi, and might break some unknown
> userspace, while the gain is limited.  Do we really want to reduce
> include/uapi/linux/a.out.h piecewise (e.g. N_BADMAG() seems to be
> unused, too), instead of keeping it until a.out support is removed
> completely?

Not really, but it looked like a magic number in the magic-number.rst
sense due to the field being "magic" and the unintuitive type naming:
I hadn't realised it's part of the on-disk format
re-examination shows that it very well may be (have been).

> Anyway, even at that point, it might be wise to keep the header file
> around, as people have expressed the desire to run a.out binaries
> through a userspace-compatibility wrapper.

Agreed. Scissor-patch that reverts the removal below.

> > A cursory glance on DCS didn't show any user code actually using this
> > value
> 
> What is DCS?

Debian Code Search; in this case my query was:
  https://codesearch.debian.net/search?q=%5CbCMAGIC%5Cb&literal=0

There's a few false positives here but all results that are using CMAGIC
to mean this CMAGIC (and aren't hurd code copied from linux)
just re-define it.

> >  Documentation/process/magic-number.rst                    | 1 -
> >  Documentation/translations/it_IT/process/magic-number.rst | 1 -
> >  Documentation/translations/zh_CN/process/magic-number.rst | 1 -
> >  Documentation/translations/zh_TW/process/magic-number.rst | 1 -
> >  include/uapi/linux/a.out.h                                | 3 ---
> >  5 files changed, 7 deletions(-)
> > 
> > diff --git a/include/uapi/linux/a.out.h b/include/uapi/linux/a.out.h
> > index 5fafde3798e5..bb15da96df2a 100644
> > --- a/include/uapi/linux/a.out.h
> > +++ b/include/uapi/linux/a.out.h
> > @@ -70,9 +70,6 @@ enum machine_type {
> >     The first page is unmapped to help trap NULL pointer references */
> >  #define QMAGIC 0314
> >
> > -/* Code indicating core file.  */
> > -#define CMAGIC 0421
> > -
> >  #if !defined (N_BADMAG)
> >  #define N_BADMAG(x)      (N_MAGIC(x) != OMAGIC         \
> >                         && N_MAGIC(x) != NMAGIC         \
> 
> Gr{oetje,eeting}s,
>                         Geert

Best,
наб

-- >8 --
Subject: [PATCH] a.out: restore CMAGIC

Part of UAPI and the on-disk format:
this means that it's not a magic number per magic-number.rst,
and it's best to leave it untouched to avoid breaking userspace
and suffer the same fate as a.out in general

Fixes: commit 53c2bd679017 ("a.out: remove define-only CMAGIC,
 previously magic number")
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 include/uapi/linux/a.out.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/a.out.h b/include/uapi/linux/a.out.h
index bb15da96df2a..5fafde3798e5 100644
--- a/include/uapi/linux/a.out.h
+++ b/include/uapi/linux/a.out.h
@@ -70,6 +70,9 @@ enum machine_type {
    The first page is unmapped to help trap NULL pointer references */
 #define QMAGIC 0314
 
+/* Code indicating core file.  */
+#define CMAGIC 0421
+
 #if !defined (N_BADMAG)
 #define N_BADMAG(x)	  (N_MAGIC(x) != OMAGIC		\
 			&& N_MAGIC(x) != NMAGIC		\
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 02/18] a.out: remove define-only CMAGIC, previously magic number

[Greg KH]

On Mon, Sep 26, 2022 at 05:15:54PM +0200, наб wrote:
> Hi!
> 
> On Mon, Sep 26, 2022 at 10:16:02AM +0200, Geert Uytterhoeven wrote:
> > Thanks for your patch, which is now commit 53c2bd679017277f
> > ("a.out: remove define-only CMAGIC, previously magic number") in
> > driver-core/driver-core-next.
> > 
> > On Fri, Sep 16, 2022 at 12:40 AM наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
> > > The last user was removed in 5.1 in
> > > commit 08300f4402ab ("a.out: remove core dumping support")
> > > but this is part of the UAPI headers, so this may want to either wait
> > > until a.out is removed entirely, or be removed from the magic number doc
> > > and silently remain in the header
> > 
> > Indeed. This is part of uapi, and might break some unknown
> > userspace, while the gain is limited.  Do we really want to reduce
> > include/uapi/linux/a.out.h piecewise (e.g. N_BADMAG() seems to be
> > unused, too), instead of keeping it until a.out support is removed
> > completely?
> 
> Not really, but it looked like a magic number in the magic-number.rst
> sense due to the field being "magic" and the unintuitive type naming:
> I hadn't realised it's part of the on-disk format
> re-examination shows that it very well may be (have been).
> 
> > Anyway, even at that point, it might be wise to keep the header file
> > around, as people have expressed the desire to run a.out binaries
> > through a userspace-compatibility wrapper.
> 
> Agreed. Scissor-patch that reverts the removal below.
> 
> > > A cursory glance on DCS didn't show any user code actually using this
> > > value
> > 
> > What is DCS?
> 
> Debian Code Search; in this case my query was:
>   https://codesearch.debian.net/search?q=%5CbCMAGIC%5Cb&literal=0
> 
> There's a few false positives here but all results that are using CMAGIC
> to mean this CMAGIC (and aren't hurd code copied from linux)
> just re-define it.
> 
> > >  Documentation/process/magic-number.rst                    | 1 -
> > >  Documentation/translations/it_IT/process/magic-number.rst | 1 -
> > >  Documentation/translations/zh_CN/process/magic-number.rst | 1 -
> > >  Documentation/translations/zh_TW/process/magic-number.rst | 1 -
> > >  include/uapi/linux/a.out.h                                | 3 ---
> > >  5 files changed, 7 deletions(-)
> > > 
> > > diff --git a/include/uapi/linux/a.out.h b/include/uapi/linux/a.out.h
> > > index 5fafde3798e5..bb15da96df2a 100644
> > > --- a/include/uapi/linux/a.out.h
> > > +++ b/include/uapi/linux/a.out.h
> > > @@ -70,9 +70,6 @@ enum machine_type {
> > >     The first page is unmapped to help trap NULL pointer references */
> > >  #define QMAGIC 0314
> > >
> > > -/* Code indicating core file.  */
> > > -#define CMAGIC 0421
> > > -
> > >  #if !defined (N_BADMAG)
> > >  #define N_BADMAG(x)      (N_MAGIC(x) != OMAGIC         \
> > >                         && N_MAGIC(x) != NMAGIC         \
> > 
> > Gr{oetje,eeting}s,
> >                         Geert
> 
> Best,
> наб
> 
> -- >8 --
> Subject: [PATCH] a.out: restore CMAGIC
> 
> Part of UAPI and the on-disk format:
> this means that it's not a magic number per magic-number.rst,
> and it's best to leave it untouched to avoid breaking userspace
> and suffer the same fate as a.out in general
> 
> Fixes: commit 53c2bd679017 ("a.out: remove define-only CMAGIC,
>  previously magic number")
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
>  include/uapi/linux/a.out.h | 3 +++
>  1 file changed, 3 insertions(+)

Thanks, I'll take this through my tree as it has the offending commit in
it.

greg k-h