public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* [Bug default/28954] New: add Linux Kernel symbol namespace support
@ 2022-03-10 13:25 gprocida at google dot com
  2022-03-10 15:52 ` Ben Woodard
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: gprocida at google dot com @ 2022-03-10 13:25 UTC (permalink / raw)
  To: libabigail

https://sourceware.org/bugzilla/show_bug.cgi?id=28954

            Bug ID: 28954
           Summary: add Linux Kernel symbol namespace support
           Product: libabigail
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: gprocida at google dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

Hi.

We have started to see Linux kernel symbol namespace changes break module
loading, without being picked up by ABI monitoring beforehand.

If a symbol has a namespace, then any module attempting to use it (run-time
load check) must have imported (compile-time) the namespace. Load failure can
be commuted to a warning but I doubt we would do that for Android.

In terms of ABI monitoring, a change to a symbol's namespace should be reported
as an ABI diff.

While it might also be nice to store the imported namespaces per module (at the
abg-corpus level), without also tracking undefined symbols this wouldn't allow
any automatic checking of kernel / module compatibility. Also, there is a
compile-time check that does that anyway. We are trying to ensure that
separately built kernel and modules will (continue to) work together.

At a minimum we need the following:

* new ELF symbol XML attribute (and possible documentation update)
* new field in ELF symbol
* symtab reader changes to populate this
* XML writer change to print this
* XML reader change to read this
* categorisation of symbol namespace changes (probably the same as for CRCs)
* symbol namespace diff generation / reporting

I'm happy to start work on this, but would welcome any early feedback.

References

https://www.kernel.org/doc/html/latest/core-api/symbol-namespaces.html
https://lwn.net/Articles/760045/
https://lpc.events/event/2/contributions/220/attachments/44/51/01._Symbol_Namespaces__Plumbers_v2.pdf

Giuliano.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bug default/28954] New: add Linux Kernel symbol namespace support
  2022-03-10 13:25 [Bug default/28954] New: add Linux Kernel symbol namespace support gprocida at google dot com
@ 2022-03-10 15:52 ` Ben Woodard
  2022-03-10 15:53 ` [Bug default/28954] " woodard at redhat dot com
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ben Woodard @ 2022-03-10 15:52 UTC (permalink / raw)
  To: gprocida at google dot com; +Cc: libabigail

I’m a little confused by this the LKM name spacing doesn’t seem to use any of the symbol mangling that C++ does. From the links that you posted it seems like the way that the kernel mangles these names is simply by putting “.<subsystem>” into the ELF linkage name. This confuses me because, I would think that this change in linkage name would show up in ABI reports but your bug description below suggests that it doesn’t. I must be missing something.

-ben

> On Mar 10, 2022, at 5:26 AM, gprocida at google dot com via Libabigail <libabigail@sourceware.org> wrote:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28954
> 
>            Bug ID: 28954
>           Summary: add Linux Kernel symbol namespace support
>           Product: libabigail
>           Version: unspecified
>            Status: UNCONFIRMED
>          Severity: normal
>          Priority: P2
>         Component: default
>          Assignee: dodji at redhat dot com
>          Reporter: gprocida at google dot com
>                CC: libabigail at sourceware dot org
>  Target Milestone: ---
> 
> Hi.
> 
> We have started to see Linux kernel symbol namespace changes break module
> loading, without being picked up by ABI monitoring beforehand.
> 
> If a symbol has a namespace, then any module attempting to use it (run-time
> load check) must have imported (compile-time) the namespace. Load failure can
> be commuted to a warning but I doubt we would do that for Android.
> 
> In terms of ABI monitoring, a change to a symbol's namespace should be reported
> as an ABI diff.
> 
> While it might also be nice to store the imported namespaces per module (at the
> abg-corpus level), without also tracking undefined symbols this wouldn't allow
> any automatic checking of kernel / module compatibility. Also, there is a
> compile-time check that does that anyway. We are trying to ensure that
> separately built kernel and modules will (continue to) work together.
> 
> At a minimum we need the following:
> 
> * new ELF symbol XML attribute (and possible documentation update)
> * new field in ELF symbol
> * symtab reader changes to populate this
> * XML writer change to print this
> * XML reader change to read this
> * categorisation of symbol namespace changes (probably the same as for CRCs)
> * symbol namespace diff generation / reporting
> 
> I'm happy to start work on this, but would welcome any early feedback.
> 
> References
> 
> https://www.kernel.org/doc/html/latest/core-api/symbol-namespaces.html
> https://lwn.net/Articles/760045/
> https://lpc.events/event/2/contributions/220/attachments/44/51/01._Symbol_Namespaces__Plumbers_v2.pdf
> 
> Giuliano.
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug default/28954] add Linux Kernel symbol namespace support
  2022-03-10 13:25 [Bug default/28954] New: add Linux Kernel symbol namespace support gprocida at google dot com
  2022-03-10 15:52 ` Ben Woodard
@ 2022-03-10 15:53 ` woodard at redhat dot com
  2022-03-10 16:25 ` gprocida at google dot com
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: woodard at redhat dot com @ 2022-03-10 15:53 UTC (permalink / raw)
  To: libabigail

https://sourceware.org/bugzilla/show_bug.cgi?id=28954

--- Comment #1 from Ben Woodard <woodard at redhat dot com> ---
I’m a little confused by this the LKM name spacing doesn’t seem to use any of
the symbol mangling that C++ does. From the links that you posted it seems like
the way that the kernel mangles these names is simply by putting “.<subsystem>”
into the ELF linkage name. This confuses me because, I would think that this
change in linkage name would show up in ABI reports but your bug description
below suggests that it doesn’t. I must be missing something.

-ben

> On Mar 10, 2022, at 5:26 AM, gprocida at google dot com via Libabigail <libabigail@sourceware.org> wrote:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28954
> 
>            Bug ID: 28954
>           Summary: add Linux Kernel symbol namespace support
>           Product: libabigail
>           Version: unspecified
>            Status: UNCONFIRMED
>          Severity: normal
>          Priority: P2
>         Component: default
>          Assignee: dodji at redhat dot com
>          Reporter: gprocida at google dot com
>                CC: libabigail at sourceware dot org
>  Target Milestone: ---
> 
> Hi.
> 
> We have started to see Linux kernel symbol namespace changes break module
> loading, without being picked up by ABI monitoring beforehand.
> 
> If a symbol has a namespace, then any module attempting to use it (run-time
> load check) must have imported (compile-time) the namespace. Load failure can
> be commuted to a warning but I doubt we would do that for Android.
> 
> In terms of ABI monitoring, a change to a symbol's namespace should be reported
> as an ABI diff.
> 
> While it might also be nice to store the imported namespaces per module (at the
> abg-corpus level), without also tracking undefined symbols this wouldn't allow
> any automatic checking of kernel / module compatibility. Also, there is a
> compile-time check that does that anyway. We are trying to ensure that
> separately built kernel and modules will (continue to) work together.
> 
> At a minimum we need the following:
> 
> * new ELF symbol XML attribute (and possible documentation update)
> * new field in ELF symbol
> * symtab reader changes to populate this
> * XML writer change to print this
> * XML reader change to read this
> * categorisation of symbol namespace changes (probably the same as for CRCs)
> * symbol namespace diff generation / reporting
> 
> I'm happy to start work on this, but would welcome any early feedback.
> 
> References
> 
> https://www.kernel.org/doc/html/latest/core-api/symbol-namespaces.html
> https://lwn.net/Articles/760045/
> https://lpc.events/event/2/contributions/220/attachments/44/51/01._Symbol_Namespaces__Plumbers_v2.pdf
> 
> Giuliano.
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.
>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug default/28954] add Linux Kernel symbol namespace support
  2022-03-10 13:25 [Bug default/28954] New: add Linux Kernel symbol namespace support gprocida at google dot com
  2022-03-10 15:52 ` Ben Woodard
  2022-03-10 15:53 ` [Bug default/28954] " woodard at redhat dot com
@ 2022-03-10 16:25 ` gprocida at google dot com
  2022-03-10 17:25 ` gprocida at google dot com
  2022-03-11 12:20 ` maennich at android dot com
  4 siblings, 0 replies; 6+ messages in thread
From: gprocida at google dot com @ 2022-03-10 16:25 UTC (permalink / raw)
  To: libabigail

https://sourceware.org/bugzilla/show_bug.cgi?id=28954

--- Comment #2 from gprocida at google dot com ---
Hi,

(In reply to Ben Woodard from comment #1)
> I’m a little confused by this the LKM name spacing doesn’t seem to use any
> of the symbol mangling that C++ does. From the links that you posted it
> seems like the way that the kernel mangles these names is simply by putting
> “.<subsystem>” into the ELF linkage name. This confuses me because, I would
> think that this change in linkage name would show up in ABI reports but your
> bug description below suggests that it doesn’t. I must be missing something.

I was confused too. It looks like that part of the scheme never happened,
probably because it was both redundant and too intrusive.

Instead we need to chase __kstrtabns_FOO to a string table entry.

Giuliano.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug default/28954] add Linux Kernel symbol namespace support
  2022-03-10 13:25 [Bug default/28954] New: add Linux Kernel symbol namespace support gprocida at google dot com
                   ` (2 preceding siblings ...)
  2022-03-10 16:25 ` gprocida at google dot com
@ 2022-03-10 17:25 ` gprocida at google dot com
  2022-03-11 12:20 ` maennich at android dot com
  4 siblings, 0 replies; 6+ messages in thread
From: gprocida at google dot com @ 2022-03-10 17:25 UTC (permalink / raw)
  To: libabigail

https://sourceware.org/bugzilla/show_bug.cgi?id=28954

--- Comment #3 from gprocida at google dot com ---
WIP patch:
https://github.com/myxoid/libabigail/commit/0227d5e85543c222282006d6fcd27bbc8bbad638

I think it has everything but the really important ksymtab decoding.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug default/28954] add Linux Kernel symbol namespace support
  2022-03-10 13:25 [Bug default/28954] New: add Linux Kernel symbol namespace support gprocida at google dot com
                   ` (3 preceding siblings ...)
  2022-03-10 17:25 ` gprocida at google dot com
@ 2022-03-11 12:20 ` maennich at android dot com
  4 siblings, 0 replies; 6+ messages in thread
From: maennich at android dot com @ 2022-03-11 12:20 UTC (permalink / raw)
  To: libabigail

https://sourceware.org/bugzilla/show_bug.cgi?id=28954

Matthias Maennich <maennich at android dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |maennich at android dot com

--- Comment #4 from Matthias Maennich <maennich at android dot com> ---
The namespaces are not part of the linkage name as that would confuse linkers,
loaders and external tools (and would also break some).

Instead the namespace is embedded in the ksymtab along with a MODVERSION CRC.
The ksymtab is basically an array of structs (struct kernel_symbol) that is
referred to from the symtab by the __ksymtab__<symbol> entries.

At module load time, every required symbol of a module is looked up in the list
of existing symbols (largely coming from vmlinux, and possibly from another
module) and loading is denied if the symbol has an attached namespace AND the
module does not declare the namespace as imported. (similarly loading is denied
upon a mismatch of the CRCs). 

Hence, the ABI breakage happens if the exported symbol newly has a namespace
attached while the consuming module does not (yet) declare the import of said
namespace.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-03-11 12:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10 13:25 [Bug default/28954] New: add Linux Kernel symbol namespace support gprocida at google dot com
2022-03-10 15:52 ` Ben Woodard
2022-03-10 15:53 ` [Bug default/28954] " woodard at redhat dot com
2022-03-10 16:25 ` gprocida at google dot com
2022-03-10 17:25 ` gprocida at google dot com
2022-03-11 12:20 ` maennich at android dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).