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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <B6A4506E3DD1F93F+Zs7Iq_EF799NyWHK@HX09040029.powercore.com.cn>
Date: Wed, 28 Aug 2024 14:50:19 +0800
From: Luming Yu <luming.yu@...ngroup.cn>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: "shenghui.qu@...ngroup.cn" <shenghui.qu@...ngroup.cn>,
	npiggin <npiggin@...il.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@...nel.org>,
	mpe <mpe@...erman.id.au>, "luming.yu" <luming.yu@...il.com>,
	杨佳龙 <jialong.yang@...ngroup.cn>,
	linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] powerpc/debug: hook to user return notifier
 infrastructure

On Wed, Aug 28, 2024 at 07:46:52AM +0200, Christophe Leroy wrote:
> Hi,
> 
> Le 28/08/2024 à 05:17, 虞陆铭 a écrit :
> > Hi,
> > 
> > it appears the little feature might require a little bit more work to find its value of the patch.
> > 
> > Using the following debug module ,  some debugging shows the TIF_USER_RETURN_NOTIFY
> > bit is propagated in __switch_to among tasks , but USER_RETURN_NOTIFY call back seems to
> > be dropped somewhere on somone who carries the bit return to user space.
> > side notes:
> > there is an issue that the module symbols is not appended to /sys/kernel/debug/tracing/available_filter_functions
> > which should be sovled first to make it easier for further debuggig.
> 
> As far as I can see, user return notifier infrastructure was implemented in
> 2009 for KVM on x86, see
> https://lore.kernel.org/all/1253105134-8862-1-git-send-email-avi@redhat.com/
> 
> Can you explain what is your usage of that infrastructure with your patch ?
> You are talking about debug, what's the added value, what is it used for ?
one example: I was thinking to live patch kernel at the moment that all cpus are
either returning to user space or going into idle. But I'm not sure if it is truly
valuable. secondly, it can help us get more accurate user/system time 
accounting via tracing rather than through sampling technique. 
The third: it could have similar usages in kvm for ppc as x86 for tsc_aux. 
etc :-)
> 
> Thanks
> Christophe
> 
> > 
> > [root@...alhost linux]# cat lib/user-return-test.c
> > #include <linux/module.h>
> > #include <linux/kernel.h>
> > #include <linux/init.h>
> > #include <linux/container_of.h>
> > #include <linux/user-return-notifier.h>
> > #include <linux/delay.h>
> > #include <linux/kthread.h>
> > #include <linux/sched.h>
> > 
> > MODULE_LICENSE("GPL");
> > 
> > 
> > struct test_user_return {
> >          struct user_return_notifier urn;
> >          bool registered;
> >          int urn_value_changed;
> >          struct task_struct *worker;
> > };
> > 
> > static struct test_user_return __percpu *user_return_test;
> > 
> > static void test_user_return_cb(struct user_return_notifier *urn)
> > {
> >          struct test_user_return *tur =
> >                  container_of(urn, struct test_user_return, urn);
> >          unsigned long flags;
> > 
> >          local_irq_save(flags);
> >          tur->urn_value_changed++;
> >          local_irq_restore(flags);
> >          return;
> > }
> > 
> > static int test_user_return_worker(void *tur)
> > {
> >          struct test_user_return *t;
> >          t = (struct test_user_return *) tur;
> >          preempt_disable();
> >          user_return_notifier_register(&t->urn);
> >          preempt_enable();
> >          t->registered = true;
> >          while (!kthread_should_stop()) {
> >                  static int err_rate = 0;
> > 
> >                  msleep (1000);
> >                  if (!test_thread_flag(TIF_USER_RETURN_NOTIFY) && (err_rate == 0)) {
> >                          pr_err("TIF_USER_RETURN_NOTIFY is lost");
> >                          err_rate++;
> >                  }
> >          }
> >          return 0;
> > }
> > static int init_test_user_return(void)
> > {
> >          int r = 0;
> > 
> >          user_return_test = alloc_percpu(struct test_user_return);
> >          if (!user_return_test) {
> >                  pr_err("failed to allocate percpu test_user_return\n");
> >                  r = -ENOMEM;
> >                  goto exit;
> >          }
> >          {
> >                  unsigned int cpu;
> >                  struct task_struct *task;
> >                  struct test_user_return *tur;
> > 
> >                  for_each_online_cpu(cpu) {
> >                          tur = per_cpu_ptr(user_return_test, cpu);
> >                          if (!tur->registered) {
> >                                  tur->urn.on_user_return = test_user_return_cb;
> >                                  task = kthread_create(test_user_return_worker,
> >                                          tur, "test_user_return");
> >                                  if (IS_ERR(task))
> >                                          pr_err("no test_user_return kthread created for cpu %d",cpu);
> >                                  else {
> >                                          tur->worker = task;
> >                                          wake_up_process(task);
> >                                  }
> >                          }
> >                  }
> >          }
> > 
> > exit:
> >          return r;
> > }
> > static void exit_test_user_return(void)
> > {
> >          struct test_user_return *tur;
> >          int i,ret=0;
> > 
> >          for_each_online_cpu(i) {
> >                  tur = per_cpu_ptr(user_return_test, i);
> >                  if (tur->registered) {
> >                          pr_info("[cpu=%d, %d] ", i, tur->urn_value_changed);
> >                          user_return_notifier_unregister(&tur->urn);
> >                          tur->registered = false;
> >                  }
> >                  if (tur->worker) {
> >                          ret = kthread_stop(tur->worker);
> >                          if (ret)
> >                                  pr_err("can't stop test_user_return kthread for cpu %d", i);
> >                  }
> >          }
> >          free_percpu(user_return_test);
> >          return;
> > }
> > 
> > module_init(init_test_user_return);
> > module_exit(exit_test_user_return);
> > 
> > ------------------ Original ------------------
> > From:  "Christophe Leroy"<christophe.leroy@...roup.eu>;
> > Date:  Tue, Feb 20, 2024 05:02 PM
> > To:  "mpe"<mpe@...erman.id.au>; "Aneesh Kumar K.V"<aneesh.kumar@...nel.org>; "虞陆铭"<luming.yu@...ngroup.cn>; "linuxppc-dev"<linuxppc-dev@...ts.ozlabs.org>; "linux-kernel"<linux-kernel@...r.kernel.org>; "npiggin"<npiggin@...il.com>;
> > Cc:  "shenghui.qu@...ngroup.cn"<shenghui.qu@...ngroup.cn>; "dawei.li@...ngroup.cn"<dawei.li@...ngroup.cn>; "ke.zhao@...ngroup.cn"<ke.zhao@...ngroup.cn>; "luming.yu"<luming.yu@...il.com>;
> > Subject:  Re: [PATCH v1 2/2] powerpc/debug: hook to user return notifier infrastructure
> > 
> > 
> > 
> > 
> > Le 20/02/2024 à 09:51, Christophe Leroy a écrit :
> > > 
> > > 
> > > Le 19/12/2023 à 07:33, Michael Ellerman a écrit :
> > > > Aneesh Kumar K.V <aneesh.kumar@...nel.org> writes:
> > > > > Luming Yu <luming.yu@...ngroup.cn> writes:
> > > > > 
> > > > > > Before we have powerpc to use the generic entry infrastructure,
> > > > > > the call to fire user return notifier is made temporarily in powerpc
> > > > > > entry code.
> > > > > > 
> > > > > 
> > > > > It is still not clear what will be registered as user return notifier.
> > > > > Can you summarize that here?
> > > > 
> > > > fire_user_return_notifiers() is defined in kernel/user-return-notifier.c
> > > > 
> > > > That's built when CONFIG_USER_RETURN_NOTIFIER=y.
> > > > 
> > > > That is not user selectable, it's only enabled by:
> > > > 
> > > > arch/x86/kvm/Kconfig:        select USER_RETURN_NOTIFIER
> > > > 
> > > > So it looks to me like (currently) it's always a nop and does nothing.
> > > > 
> > > > Which makes me wonder what the point of wiring this feature up is :)
> > > > Maybe it's needed for some other feature I don't know about?
> > > > 
> > > > Arguably we could just enable it because we can, and it currently does
> > > > nothing so it's unlikely to break anything. But that also makes it
> > > > impossible to test the implementation is correct, and runs the risk that
> > > > one day in the future when it does get enabled only then do we discover
> > > > it doesn't work.
> > > 
> > > Opened an "issue" for the day we need it:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FKSPP%2Flinux%2Fissues%2F348&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cd9e9b6315413430cfba108dcc7100633%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638604118862628419%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=iIYQTodb9zrGdfmzvnZrIVZ%2BKh2qZjMjT29ddkUpGIw%3D&reserved=0
> > 
> > Correct one is https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinuxppc%2Fissues%2Fissues%2F477&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cd9e9b6315413430cfba108dcc7100633%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638604118862637095%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=aJRVdRWu%2F7NQ13jQ6yFLBynXIIfPPPQ3nS4FxiXGNyw%3D&reserved=0
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ