| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 15 Apr 2015 10:30:31 +0200 (CEST)
From: Miroslav Benes <mbenes@...e.cz>
To: Minfei Huang <mhuang@...hat.com>
cc: Petr Mladek <pmladek@...e.cz>,
Minfei Huang <minfei.huang@...mail.com>,
Josh Poimboeuf <jpoimboe@...hat.com>, sjenning@...hat.com,
jkosina@...e.cz, vojtech@...e.cz, live-patching@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] livepatch: Fix the bug if the function name is larger
than KSYM_NAME_LEN-1
On Wed, 15 Apr 2015, Minfei Huang wrote:
> On 04/14/15 at 08:41pm, Petr Mladek wrote:
> > On Wed 2015-04-15 01:01:39, Minfei Huang wrote:
> > > On 04/14/15 at 06:27pm, Petr Mladek wrote:
> > > > On Tue 2015-04-14 23:55:36, Minfei Huang wrote:
> > > > > On 04/14/15 at 10:11P, Josh Poimboeuf wrote:
> > > > > > On Tue, Apr 14, 2015 at 01:45:49PM +0800, Minfei Huang wrote:
> > > > > > > On 04/14/15 at 12:32P, Josh Poimboeuf wrote:
> > > > > > > > On Tue, Apr 14, 2015 at 01:29:50PM +0800, Minfei Huang wrote:
> > > > > > > > >
> > > > > > > > > For end user, they may know litter about restriction of kallsyms and
> > > > > > > > > livepatch. How can they know the restriction that function name is
> > > > > > > > > limited to 127?
> > > > > > > >
> > > > > > > > As I mentioned above, I think kallsyms.c should fail the build if it
> > > > > > > > encounters a symbol longer than KSYM_NAME_LEN.
> > > > > > > >
> > > > > > >
> > > > > > > I dont think it is a good idea to handle this case like that. The
> > > > > > > function name is only for human recognization. Why the compiler fails
> > > > > > > to build it?
> > > > > >
> > > > > > Well, the function name isn't only for human recognition. kpatch-build
> > > > > > generates patch modules automatically. It assumes that the compiled
> > > > > > function name matches the kallsyms name. And I'd guess that a lot of
> > > > > > other code (both in-kernel and user space tools) make the same
> > > > > > assumption.
> > > > > >
> > > > > > Not to mention that most humans would also make the same assumption...
> > > > >
> > > > > Yes. The assumption is correct for most case.
> > > > >
> > > > > It is significance for livepatch to support extra module, because in my
> > > > > opinion kernel is more stable than the third module.
> > > > >
> > > > > So it is more important, if the livepatch can patch all sorts of patch.
> > > > > For dynamic function name, I think it is simple to avoid it.
> > > >
> > > > Do you have some really existing module with such a crazy long
> > > > function names or is this debate pure theoretical, please?
> > > >
> > >
> > > No, I do not have such running module which function name is exceed to
> > > 127.
> > >
> > > Again, we can not predict what end user do to name the function name. I
> > > think the overlength function name is valid for linux kernel, if the
> > > module can be installed.
> >
> > My position on this is that using >127 length function names is
> > insane. I would be scared to use such a module on a production system.
> > If we refuse patching, we actually do a favor for the user.
> > Instead of fixing live patch for such a scenario, we should suggest
> > the user to use more trustful modules.
>
> Yes, the function name can be changed, before the extra module is
> installed to the production system.
>
> We discuss around and around, there are still some confusion with it.
>
> 1) How does end user know that livepatch can _not_ support the function
> which length is larger than 127. We can not enforce the end user
> to know the livepatch and kallsyms code in detail.
> 2) How does end user use livepatch to patch running extra module, once
> the module is running in the production system, if the function name
> is insane.
> 3) The error message is ambiguity, if we try to patch the overlength
> function. We can give the error message clearly, once the function
> name is overlength.
>
> I think it is better that we can take more time on the people who will
> use livepatch frequently.
Just my two cents, even if we admit that such change is worth it (and I
am still not convinced that it is the case), I think it would make sense
to fix it somewhere in kallsyms as Josh proposed. I suspect that when
function names longer than KSYM_NAME_LEN were common there would be many
similar problems elsewhere in the kernel.
That is you can prepare a patch to kallsyms and submit it there. Not sure
who is the maintainer but he might have an opinion about this...
Thanks,
Miroslav
>
> Attaching a patch to make error message explictly for the overlength
> function name.
>
> >From d46a230499303657a914d6939c3afbeff906796c Mon Sep 17 00:00:00 2001
> From: Minfei Huang <minfei.huang@...mail.com>
> Date: Wed, 15 Apr 2015 10:02:43 +0800
> Subject: [PATCH] livepatch: Make error message explicitly for the overlength
> function name
>
> For not, livepatch do not support the function which name is larger than
> KSYM_NAME_LEN-1. It may be confusion user with error message
> "livepatch: symbol 'xxx(function name)' not found in symbol table".
>
> Make error message explicitly for overlength issue.
>
> Signed-off-by: Minfei Huang <minfei.huang@...mail.com>
> ---
> kernel/livepatch/core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 3f9f1d6..d1f2404 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -789,6 +789,12 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
> return -ENOMEM;
>
> for (func = obj->funcs; func->old_name; func++) {
> + if (strlen(func->old_name) > (KSYM_NAME_LEN-1)) {
> + pr_err("%s is overlength, the max to be supported is %d\n",
> + func->old_name, KSYM_NAME_LEN-1);
> + ret = -EINVAL;
> + goto free;
> + }
> ret = klp_init_func(obj, func);
> if (ret)
> goto free;
> --
> 2.2.2
>
> >
> > Best Regards,
> > Petr
> --
> To unsubscribe from this list: send the line "unsubscribe live-patching" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists