Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Nick Alcock <nick.alcock@oracle.com>
To:     Kees Cook <keescook@chromium.org>
Cc:     jeyu@kernel.org, masahiroy@kernel.org,
        linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
        arnd@arndb.de, eugene.loh@oracle.com, kris.van.hees@oracle.com
Subject: Re: [PATCH v5] kallsyms: new /proc/kallmodsyms with builtin modules
References: <20210929215154.300692-1-nick.alcock@oracle.com>
        <202109291629.81106C83D@keescook>
Emacs:  or perhaps you'd prefer Russian Roulette, after all?
Date:   Thu, 30 Sep 2021 18:19:55 +0100
In-Reply-To: <202109291629.81106C83D@keescook> (Kees Cook's message of "Wed,
        29 Sep 2021 16:34:39 -0700")
Message-ID: <875yui0ymc.fsf@esperi.org.uk>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2.50 (gnu/linux)
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?CTs/FdIuwidMkzqNBxUhsAKa22cF6Sraeh2NOVxr51vakgRa7U2jwE2dw6xM?=
 =?us-ascii?Q?LdZ1mYqBLQWtGj7J0fCMQaaWc5ohyw//qqY7c9axN6UXsb0hZrraHZmoKzum?=
 =?us-ascii?Q?f8BAOaaETYG82SIrhSO2BGjJty3Yh3PDFUPniM9AdBJ1TQlXNQ1p1QofzHud?=
 =?us-ascii?Q?szTCt4gvVisGJGJgBGOlrf9EaKlFG4BsmpHO2wCdOimKmpPCDxd+fA2IKqjs?=
 =?us-ascii?Q?UD0EePtcjruvbxUVqLKeSvVBW1oim3sw1GkK3Q+BKYzgwqQdBak+PNs/Gian?=
 =?us-ascii?Q?RRqrO3aerijceLt73SnG3bjNrT6tkFtl0m+6ktPoQj1pY5mYiyr/5BWCNfz0?=
 =?us-ascii?Q?TnCn4vG2lylt61PAEWzglsMfWuJ3W45UjXjkLRIaMCHzB4u/yPMIRXshLa9D?=
 =?us-ascii?Q?AgebAs1g6THjsYq9rrZ56zUUUl41o1Q3NcUbA59roYbdFk5ZuObZ6yo6ux4C?=
 =?us-ascii?Q?NF3yll2q46DBdZfk3PaGzztW1dApt6I8KJSEfcRAZl8NmAdAkePWf1cstYMv?=
 =?us-ascii?Q?1HDWEG7h5OVqp7/sT9UZc8gJYvi3dmqCG84S7Jt15v++L7uHktsBmyDlVQDH?=
 =?us-ascii?Q?Tg/e2sU/sjUxHc1vxFogxBGOxpjcK6RdvuREX74BSeW+fqWuWe40zmBGbYiH?=
 =?us-ascii?Q?Lf/9Nmx/SjathCg3Nvm94bCmi8Xd/xqJ3IJv2350ZE0sEpj+MpaPVI5znv5y?=
 =?us-ascii?Q?WDTCkBZ+bnLdD7XrHBoG28pTc6/6CctPkNF69gj3Kc1yyAc2tL/0VCLjzlbG?=
 =?us-ascii?Q?D2sTRFOOBGOOROLZ8bDkQPgDzXbb2ptI0ta9SS9q/fS7YupfPw5VPPHzcmb2?=
 =?us-ascii?Q?wKNrmXjSayCu9HLBrE+GcmD6bKcsaltLwWLmpjpbQSzgcSGs7n8tWBavhi1Q?=
 =?us-ascii?Q?K6a1IrEBzaKJaGwetcZ1kPe1zLldhGiePg2XvvvcI5ER0wTAI88Oack28vlI?=
 =?us-ascii?Q?DjzV9iLfMxLpL7LtGdsio8GX0FKuvpI4TtEqK3L6ncH4yHP3AylMiEs3UHJ7?=
 =?us-ascii?Q?0Nac0LsqkXQrLyMU1M5PKshuyPH6lE5nN80SQKRs5BvuLbtIQ8UaJGUHDyau?=
 =?us-ascii?Q?uKoyd23o9br7qtn0yMONxH+sX/clMzj49TknMobjdr5WFQAV3EJNIHgnKAkF?=
 =?us-ascii?Q?IWADqQtb38sQcNiUdGNZOXvsksrSnj7RXqow6srCAhtcTCDK0mZyjjOBLYdZ?=
 =?us-ascii?Q?ITRrJbMQNVyL+XKnkbzux7lE+52c9DipHb8MuJD7ZYcK3wK1Cchz9b0xVYB5?=
 =?us-ascii?Q?jnAvv/q56ul17NtQNkr5PAJTN55bENYe/P+slZ09fHAEfXCj34aNMROloIeM?=
 =?us-ascii?Q?yxGHft16uhgjacbSQnz2qGEBjRF4azG/47BhwzViGlEIlA=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 429945e4-2517-4aa3-0db5-08d98436891d
X-MS-Exchange-CrossTenant-AuthSource: SA2PR10MB4715.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2021 17:20:01.5408
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: oL5zwv2CYL3yHr14ZAHbAR4T+p5K8RpUIzDZTM88zMIpUnvKVutjcpYorqI6CChnGzSlwUJe+SIhqVJCDrZtrA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN4PR10MB5558
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10123 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 mlxscore=0 spamscore=0 adultscore=0 bulkscore=0 phishscore=0
 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2109230001 definitions=main-2109300105
X-Proofpoint-GUID: Cv2qZ1H2lxn7KT27mm4kRCqC3hbQsHie
X-Proofpoint-ORIG-GUID: Cv2qZ1H2lxn7KT27mm4kRCqC3hbQsHie
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 30 Sep 2021, Kees Cook said:

> On Wed, Sep 29, 2021 at 10:51:47PM +0100, Nick Alcock wrote:
>> It would be useful if there were a mapping between kernel symbol and module
>> name that only changed when the kernel source code is changed.  This mapping
>> should not change simply because a module becomes built into the kernel.
>> 
>> It might also be useful if there were reliable symbol size information to
>> determine whether an address is within a symbol or outside it, especially
>> given that there could be huge gaps between symbols.
>
> This is a pretty cool series, but I'm left wondering "for what reason?" :)
> Perhaps I missed the specific rationale; there was a lot to run. ;)

The underlying rationale that the earliest, crude version of this patch
was written for is that DTrace allows symbol and type names to be
module-qualified (e.g. hid`names) and we don't want people's scripts to
break just because they recompile the kernel to make things a module
versus not. But this problem is not DTrace-specific: given that names
are not unique across the kernel when things are built as modules, many
tracing tools that introspect the values of kernel symbols etc might
want to let users qualify names with the modules they apply to, and more
or less none of those are going to want a script that asks for "foo in
the bar module" to suddenly become invalid because the user changed
CONFIG_FOO=m to CONFIG_FOO=y.

Obviously, if they change it to CONFIG_FOO=n, any script that uses names
from FOO is going to break: there's nothing we can do about that. But
not breaking the m/y case seems like a simple quality-of- implementation
issue to make it easier to redistribute scripts to anyone using FOO no
matter what their module/non-module configuration, and a nice easy one
to fix, if only we could easily tell what the module/symbol mapping was.
And with this patch we can (at minimal cost).

(I have also found it to be useful for by-hand stuff, when I can't
remember where the heck some symbol with a non-qualified name is,
usually a file-scope variable: it's faster to grep kallmodsyms than to
do a tags lookup, let alone a grep, if I'm debugging something I know is
loaded into the kernel right now. Which often one is if one is
reproducing the bug... and often the module a symbol is part of is
informative. This is, obviously, crude, but it's still more useful to
have more module names there than fewer if one is doing this sort of
thing.)


(As an aside, this is not all a tracing system needs to do to get this
sort of thing right, particularly where types are concerned: it's still
possible to have conflicting type names across translation units within
a single module. CTF deals with that case perfectly well via child CTF
dicts, and GNU ld splits ambiguously-defined types out so that the user
can say "foo_t in the bar module in wombat.c" in the rare cases where
ambiguity exists.)

> It would be useful, sure, but is there something that does, in fact,
> need this, or would like this if it were available? Since this provides
> a userspace API, what would be consuming that API? For example, when
> Syscall User Dispatch was added, it was clear it was for Wine[1].

We have a long-term consumer in DTrace (e.g.
<https://github.com/oracle/dtrace-utils/blob/dev/libdtrace/dt_module.c#L1323>).
(But since we control this consumer we are quite happy to change the
format of kallmodsyms, integrate it with kallsyms so that kallsyms just
provides the same information if the file format break were acceptable,
or whatever you think most sensible. We just want this information
somehow. :) e.g. the code there doesn't yet handle the
symbol-in-multiple-modules case: I'll have to add that.)

But this feels useful enough to me that I'm hoping that more consumers
will grow.  I'm sure SystemTap would find it useful, and I'm hoping
bpftrace etc might as well once that starts to handle modules. They're
all going to need some sort of namespace-qualification and then they're
all going to run into the same script-stability problem as soon as users
try to share scripts at all. We can make that problem largely go away :)

-- 
NULL && (void)