Re: [PATCH 2/2] livepatch: Fix the bug if the function name is larger than KSYM_NAME_LEN-1

[Minfei Huang]

On 04/15/15 at 12:24pm, Justin Keller wrote:
> I agree that 128+ function names are bad and there is no need for such
> long names, epically for compatibility with 80 char/line displays. I
> cannot think of an example even near as long as 127 characters. Are
> there even such long function names anywhere in the kernel?
> 
> Justin
> 

Yes, the overlength function name is insane. For the upstream source, we
can never meet such case, because we can control the code to be merged
or not.

But there is an exception for extra module which composes by end user.

IMO, livepatch is a tool which helps to inject new function to replace
the old one. Since the patahed patch is from userspace, it may be not
trusted. Do all of the condition judgement we can to confirm it does
not corrupt the kernel.

Thanks
Minfei

> On Wed, Apr 15, 2015 at 7:58 AM, Miroslav Benes <mbenes@xxxxxxx> wrote:
> > On Wed, 15 Apr 2015, Minfei Huang wrote:
> >
> >> On 04/15/15 at 10:30P, Miroslav Benes wrote:
> >> > On Wed, 15 Apr 2015, Minfei Huang wrote:
> >> >
> >> > >
> >> > > 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
> >>
> >> Ohhh...
> >>
> >> Fixing kallsyms to restrict the function name length maybe is not a good
> >> idea. I have no idea how we should do this, except for the coding
> >> problems.
> >
> > Well we do it now via scripts/kallsyms.c when vmlinux is built. Try it. We
> > apparently do not do it when kernel modules are built out of the tree (as
> > you demonstrated before). So the question is whether we should do it also
> > there. That is one thing we try to tell you.
> >
> > The other one is that 128 characters long function names are insane.
> > Probably that is what KSYM_NAME_LEN is for in the first place. Maybe you
> > could even try to add the check to checkpatch.pl.
> >
> >> > 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
> >>
> >> Hold on, I get a scenario that livepatch may do fatal error. I am fine
> >> that livepatch do not support overlength function name, because it can
> >> not corrupt the kernel.
> >>
> >> Once there is a function name A is larger than 127, and another function
> >> name B is as longer as 127, it is disaster that we want to patch
> >> function B, if function name of first 127 is same between A and B.
> >
> > True, but see above.
> >
> >> Livepatch may find the function of A to patch it. So this patch(2/2) may
> >> be needed to fix the issue.
> >
> > Hm, but this patch is not the solution for the issue, or is it? You would
> > check only those first KSYM_NAME_LEN characters, but that would not
> > differentiate between A and B. Or maybe I do not follow.
> >
> > Thanks
> > Miroslav
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/