From: Miroslav Benes <mbenes@suse.cz> To: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Chris J Arges <chris.j.arges@canonical.com>, live-patching@vger.kernel.org, jeyu@redhat.com, Seth Jennings <sjenning@redhat.com>, Jiri Kosina <jikos@kernel.org>, Vojtech Pavlik <vojtech@suse.com>, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] livepatch: old_name.number scheme in livepatch sysfs directory Date: Wed, 4 Nov 2015 10:52:52 +0100 (CET) [thread overview] Message-ID: <alpine.LNX.2.00.1511041047200.5621@pobox.suse.cz> (raw) In-Reply-To: <20151103165058.GM27488@treble.redhat.com> On Tue, 3 Nov 2015, Josh Poimboeuf wrote: > On Tue, Nov 03, 2015 at 05:09:48PM +0100, Miroslav Benes wrote: > > On Tue, 3 Nov 2015, Josh Poimboeuf wrote: > > > > > On Tue, Nov 03, 2015 at 11:52:08AM +0100, Miroslav Benes wrote: > > > > > > > > There is a problem which I missed before. klp_init_func() is called before > > > > klp_find_verify_func_addr() in klp_init_object(). This means that > > > > func->old_addr is either not verified yet or worse it is still 0. This > > > > means that klp_get_func_pos_callback() never returns 1 and is thus called > > > > on each symbol. So if you for example patched cmdline_proc_show the > > > > resulting directory in sysfs would be called cmdline_proc_show,1 because > > > > addr is never matched. Had old_addr been specified the name would have > > > > been probably correct, but not for sure. > > > > > > > > This should be fixed as well. > > > > > > Even worse, klp_init_func() can be called even if the object hasn't been > > > loaded. In that case there's no way to know what the value of n is, and > > > therefore no way to reliably create the sysfs entry. > > > > Ah, right. > > > > > Should we create "func,n" in klp_init_object_loaded() instead of > > > klp_init_func()? > > > > So that the function entries in sysfs would be created only when the > > object is loaded? Well, why not, but in that case it could easily confuse > > the user. > > Maybe, but I think it would be fine if we document it. It should only > be relied on by tools, anyway. Agreed. > > Object entry would be empty for not loaded object. I would not > > dare to propose to remove such object entries. It would make things worse. > > Why would removing an empty object entry make things worse? I think it all comes down to a question whether the sysfs entries say what a patch is capable to patch or what this patch is currently patching in the system. I am inclined to the former so the removal would make me nervous. But I am not against the second approach. We are still in testing mode as far as sysfs is concerned so we can try even harsh changes and see how it's gonna go. > > So maybe we could introduce an attribute in sysfs object entry which would > > say if the object is loaded or not. Or something like that. > > Hm, I'm not sure I see how this would help. Hopefully I cleared this up with the above. Miroslav
WARNING: multiple messages have this Message-ID (diff)
From: Miroslav Benes <mbenes-AlSwsSmVLrQ@public.gmane.org> To: Josh Poimboeuf <jpoimboe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Cc: Chris J Arges <chris.j.arges-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>, live-patching-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, jeyu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Seth Jennings <sjenning-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Jiri Kosina <jikos-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Vojtech Pavlik <vojtech-IBi9RG/b67k@public.gmane.org>, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH v2] livepatch: old_name.number scheme in livepatch sysfs directory Date: Wed, 4 Nov 2015 10:52:52 +0100 (CET) [thread overview] Message-ID: <alpine.LNX.2.00.1511041047200.5621@pobox.suse.cz> (raw) In-Reply-To: <20151103165058.GM27488-8wJ5/zUtDR0XGNroddHbYwC/G2K4zDHf@public.gmane.org> On Tue, 3 Nov 2015, Josh Poimboeuf wrote: > On Tue, Nov 03, 2015 at 05:09:48PM +0100, Miroslav Benes wrote: > > On Tue, 3 Nov 2015, Josh Poimboeuf wrote: > > > > > On Tue, Nov 03, 2015 at 11:52:08AM +0100, Miroslav Benes wrote: > > > > > > > > There is a problem which I missed before. klp_init_func() is called before > > > > klp_find_verify_func_addr() in klp_init_object(). This means that > > > > func->old_addr is either not verified yet or worse it is still 0. This > > > > means that klp_get_func_pos_callback() never returns 1 and is thus called > > > > on each symbol. So if you for example patched cmdline_proc_show the > > > > resulting directory in sysfs would be called cmdline_proc_show,1 because > > > > addr is never matched. Had old_addr been specified the name would have > > > > been probably correct, but not for sure. > > > > > > > > This should be fixed as well. > > > > > > Even worse, klp_init_func() can be called even if the object hasn't been > > > loaded. In that case there's no way to know what the value of n is, and > > > therefore no way to reliably create the sysfs entry. > > > > Ah, right. > > > > > Should we create "func,n" in klp_init_object_loaded() instead of > > > klp_init_func()? > > > > So that the function entries in sysfs would be created only when the > > object is loaded? Well, why not, but in that case it could easily confuse > > the user. > > Maybe, but I think it would be fine if we document it. It should only > be relied on by tools, anyway. Agreed. > > Object entry would be empty for not loaded object. I would not > > dare to propose to remove such object entries. It would make things worse. > > Why would removing an empty object entry make things worse? I think it all comes down to a question whether the sysfs entries say what a patch is capable to patch or what this patch is currently patching in the system. I am inclined to the former so the removal would make me nervous. But I am not against the second approach. We are still in testing mode as far as sysfs is concerned so we can try even harsh changes and see how it's gonna go. > > So maybe we could introduce an attribute in sysfs object entry which would > > say if the object is loaded or not. Or something like that. > > Hm, I'm not sure I see how this would help. Hopefully I cleared this up with the above. Miroslav
next prev parent reply other threads:[~2015-11-04 9:52 UTC|newest] Thread overview: 104+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-11-02 17:58 [PATCH] livepatch: old_name.number scheme in livepatch sysfs directory Chris J Arges 2015-11-02 17:58 ` Chris J Arges 2015-11-02 19:15 ` Jessica Yu 2015-11-02 19:52 ` [PATCH] " Josh Poimboeuf 2015-11-02 19:52 ` Josh Poimboeuf 2015-11-02 20:16 ` Chris J Arges 2015-11-02 20:32 ` Josh Poimboeuf 2015-11-02 22:59 ` [PATCH v2] " Chris J Arges 2015-11-02 22:59 ` Chris J Arges 2015-11-03 9:50 ` Miroslav Benes 2015-11-03 15:03 ` Josh Poimboeuf 2015-11-03 10:52 ` Miroslav Benes 2015-11-03 10:52 ` Miroslav Benes 2015-11-03 12:44 ` Petr Mladek 2015-11-03 12:44 ` Petr Mladek 2015-11-03 15:03 ` Josh Poimboeuf 2015-11-03 15:03 ` Josh Poimboeuf 2015-11-03 19:57 ` Jiri Kosina 2015-11-03 19:57 ` Jiri Kosina 2015-11-03 20:06 ` Josh Poimboeuf 2015-11-03 20:06 ` Josh Poimboeuf [not found] ` <1447259366-7055-1-git-send-email-chris.j.arges@canonical.com> 2015-11-11 16:28 ` [PATCH 1/3 v4] livepatch: add old_sympos as disambiguator field to klp_func Chris J Arges 2015-11-11 17:43 ` Josh Poimboeuf 2015-11-12 10:22 ` Miroslav Benes 2015-11-11 16:29 ` [PATCH 2/3 v4] livepatch: add old_sympos as disambiguator field to klp_reloc Chris J Arges 2015-11-11 16:39 ` Chris J Arges 2015-11-11 17:57 ` Josh Poimboeuf 2015-11-12 14:31 ` Petr Mladek 2015-11-12 19:19 ` Josh Poimboeuf 2015-11-13 13:54 ` Petr Mladek 2015-11-13 16:59 ` Josh Poimboeuf 2015-11-16 12:11 ` Petr Mladek 2015-11-12 10:23 ` Miroslav Benes 2015-11-11 16:29 ` [PATCH 3/3 v4] livepatch: old_name,number scheme in livepatch sysfs directory Chris J Arges 2015-11-11 18:01 ` Josh Poimboeuf 2015-11-11 18:01 ` Josh Poimboeuf [not found] ` <1447347595-30728-1-git-send-email-chris.j.arges@canonical.com> 2015-11-12 16:59 ` [PATCH 1/4 v5] livepatch: add old_sympos as disambiguator field to klp_func Chris J Arges 2015-11-12 19:45 ` Josh Poimboeuf 2015-11-13 9:56 ` Jiri Kosina 2015-11-13 10:14 ` Miroslav Benes 2015-11-13 16:27 ` Petr Mladek 2015-11-12 16:59 ` [PATCH 2/4 v5] livepatch: Simplify code for relocated external symbols Chris J Arges 2015-11-13 10:24 ` Miroslav Benes 2015-11-13 13:55 ` Petr Mladek 2015-11-12 16:59 ` [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc Chris J Arges 2015-11-12 19:50 ` Josh Poimboeuf 2015-11-13 16:42 ` Petr Mladek 2015-11-12 16:59 ` [PATCH 4/4 v5] livepatch: function,sympos scheme in livepatch sysfs directory Chris J Arges [not found] ` <1447431804-18786-1-git-send-email-chris.j.arges@canonical.com> 2015-11-13 16:23 ` [PATCH 1/3 v6] livepatch: add old_sympos as disambiguator field to klp_func Chris J Arges 2015-11-13 16:23 ` [PATCH 2/3 v6] livepatch: add sympos as disambiguator field to klp_reloc Chris J Arges 2015-11-13 16:23 ` [PATCH 3/3 v6] livepatch: function,sympos scheme in livepatch sysfs directory Chris J Arges [not found] ` <1447693391-10065-1-git-send-email-chris.j.arges@canonical.com> 2015-11-16 17:03 ` [PATCH 1/3 v7] livepatch: add old_sympos as disambiguator field to klp_func Chris J Arges 2015-11-16 21:59 ` Jiri Kosina 2015-11-17 14:29 ` Chris J Arges 2015-11-19 10:02 ` Jiri Kosina 2015-11-16 17:03 ` [PATCH 2/3 v7] livepatch: add sympos as disambiguator field to klp_reloc Chris J Arges 2015-11-18 9:56 ` Miroslav Benes 2015-11-18 14:01 ` Josh Poimboeuf 2015-11-18 16:47 ` Petr Mladek 2015-11-18 16:37 ` Petr Mladek 2015-11-18 16:39 ` Chris J Arges 2015-11-18 16:55 ` Petr Mladek 2015-11-18 20:34 ` Jiri Kosina 2015-11-16 17:03 ` [PATCH 3/3 v7] livepatch: function,sympos scheme in livepatch sysfs directory Chris J Arges 2015-11-20 17:25 ` [PATCH 0/3 v8] livepatch: disambiguate symbols with the same name Chris J Arges 2015-11-20 17:25 ` [PATCH 1/3 v8] livepatch: add old_sympos as disambiguator field to klp_func Chris J Arges 2015-11-23 9:47 ` Miroslav Benes 2015-11-20 17:25 ` [PATCH 2/3 v8] livepatch: add sympos as disambiguator field to klp_reloc Chris J Arges 2015-11-23 9:52 ` Miroslav Benes 2015-11-30 20:46 ` Chris J Arges 2015-12-01 1:17 ` Josh Poimboeuf 2015-11-20 17:25 ` [PATCH 3/3 v8] livepatch: function,sympos scheme in livepatch sysfs directory Chris J Arges 2015-11-23 9:47 ` Miroslav Benes 2015-12-02 2:40 ` [PATCH 0/3 v9] livepatch: disambiguate symbols with the same name Chris J Arges 2015-12-02 2:40 ` [PATCH 1/3 v9] livepatch: add old_sympos as disambiguator field to klp_func Chris J Arges 2015-12-02 2:40 ` [PATCH 2/3 v9] livepatch: add sympos as disambiguator field to klp_reloc Chris J Arges 2015-12-14 12:45 ` Miroslav Benes 2015-12-02 2:40 ` [PATCH 3/3 v9] livepatch: function,sympos scheme in livepatch sysfs directory Chris J Arges 2015-12-02 15:28 ` [PATCH 0/3 v9] livepatch: disambiguate symbols with the same name Petr Mladek 2015-12-03 17:59 ` Josh Poimboeuf 2015-12-03 22:04 ` Jiri Kosina 2015-11-03 14:58 ` [PATCH v2] livepatch: old_name.number scheme in livepatch sysfs directory Josh Poimboeuf 2015-11-03 16:09 ` Miroslav Benes 2015-11-03 16:50 ` Josh Poimboeuf 2015-11-03 20:42 ` Chris J Arges 2015-11-03 20:42 ` Chris J Arges 2015-11-04 9:52 ` Miroslav Benes [this message] 2015-11-04 9:52 ` Miroslav Benes 2015-11-04 16:03 ` Josh Poimboeuf 2015-11-04 16:17 ` Chris J Arges 2015-11-04 16:17 ` Chris J Arges 2015-11-05 15:18 ` Miroslav Benes 2015-11-05 15:56 ` Josh Poimboeuf 2015-11-05 16:07 ` Chris J Arges 2015-11-09 16:16 ` [PATCH v3] livepatch: old_name,number " Chris J Arges 2015-11-09 16:16 ` Chris J Arges 2015-11-09 20:56 ` Josh Poimboeuf 2015-11-09 23:01 ` Chris J Arges 2015-11-09 23:01 ` Chris J Arges 2015-11-10 4:54 ` Josh Poimboeuf 2015-11-10 8:49 ` Miroslav Benes 2015-11-10 13:40 ` Josh Poimboeuf 2015-11-10 13:40 ` Josh Poimboeuf 2015-11-10 9:02 ` Miroslav Benes
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.LNX.2.00.1511041047200.5621@pobox.suse.cz \ --to=mbenes@suse.cz \ --cc=chris.j.arges@canonical.com \ --cc=jeyu@redhat.com \ --cc=jikos@kernel.org \ --cc=jpoimboe@redhat.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=sjenning@redhat.com \ --cc=vojtech@suse.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.