On Thu, Jul 16, 2015 at 01:07:13AM +0800, Minfei Huang wrote: > > > On Jul 15, 2015, at 21:24, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote: > > > > On Wed, Jul 15, 2015 at 05:02:19PM +0800, Minfei Huang wrote: > >> On 07/15/15 at 10:53P, Miroslav Benes wrote: > >>> On Tue, 14 Jul 2015, Josh Poimboeuf wrote: > >>> > >>>> On Tue, Jul 14, 2015 at 10:57:46PM +0200, Jiri Kosina wrote: > >>>>> On Tue, 14 Jul 2015, Josh Poimboeuf wrote: > >>>>> > >>>>>> Here's another issue [1] which Jessica found. It affects the livepatch > >>>>>> sysfs API, so we should probably fix it soon before we get actual users. > >>>>>> > >>>>>> Here's our current sysfs layout for displaying patched function names: > >>>>>> > >>>>>> /sys/kernel/livepatch/<patch>/<object>/<func> > >>>>>> > >>>>>> A problem occurs when patching two distinct functions which have the > >>>>>> same name: > >>>>> > >>>>> ... and are both built-in. > >>>> > >>>> True. > >>> > >>> I think we even discussed this issue a while ago. Can't find it now in the > >>> archives and don't remember the outcome... > >>> > >>>>>> sysfs: cannot create duplicate filename '/kernel/livepatch/kpatch_s_next_ambig/vmlinux/s_next' > >>>>>> > >>>>>> Does anybody rely on the "func" sysfs entry? > >>>>> > >>>>> Well, our sysfs ABI is documented in testing/, so if worse comes to worst, > >>>>> we should be able to fix it in a backwards incompatible way without being > >>>>> called the bad guys breaking userspace. > >>>> > >>>> Ok. Hopefully it doesn't come to that :-) > >>>> > >>>>> > >>>>>> I suppose our options are to either remove "func" completely or replace > >>>>>> it with something more unique like the function address. > >>>>> > >>>>> How about extending the attribute to consist of both name and the address. > >>>>> That will automatically provide disambiguation, and would still maintain > >>>>> human readability. > >>>> > >>>> Sounds fine to me. Something like: > >>>> > >>>> "cmdline_proc_show,0xffffffff812904c0" ? > >>> > >>> Unfortunately this would be shown even to non-root users and is > >>> information leak (if kptr_restrict is set to non-zero). > >> > >> How about append the patched function name in the sysfs. It may be more > >> clear to show the relationship between function and patched function. > > > > But that still doesn't guarantee a unique name if all the functions are > > static. > > I think this approach can fix this issue, since function name is unique in one module. But function names aren't necessarily unique in a module. -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
