Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190925112014.cjb23adhmpgcszou@gusev>
Date: Wed, 25 Sep 2019 14:20:14 +0300
From: "Alexander V. Gusev" <agusev@...ralinux.ru>
To: lkrg-users@...ts.openwall.com
Subject: Re: lkrg-0.7 lkrg.hide=1 causes LOST MODULE

On Tue, Sep 24, 2019 at 03:57:21AM +0200, Adam Zabrocki wrote:

> > Well, I do have _some_ time and energy. How exactly would you like me
> > to cooperate?
> 
> If you are willing to help with changing LKRG's coding style to match Linux 
> kernel, I'm happy to cooperate with you directly. There are a few possible 
> ways:
> 
> 1. We can chat online e.g. via IRC (freenode).
> 2. We can chat privately via email.
> 3. We can chat via this mailing list.

I'd prefer private email.
 
> If you prepare patch I'm happy (and probably Alexander as well) to review it. 
> After review we have a few options:
> 
> 1. You can submit Pull Request to the oficial repo which I would merge.
> 2. You can submit your patch/diff to the official mailing list. I would pick it 
> up and commit by myself.
> 3. You can directly sent me your patch via email and I will manually merge and 
> commit it.

I think we should start with 3 and then move on to 1 if it all goes well.

> > Why not? I am loading lkrg in initrd, as early as possible, then at some
> > point during bootup the systemd unit starts and continues setting up
> > the module via sysctl. In this case it is more convenient to immediately
> > adjust all settings through module parameters, and it is probably more
> > secure.
> > 
> 
> Hiding feature is "experimental" and there is still a way to find it from the 
> user-mode (e.g. via sysctl / proc interface). It is possible to close that gap 
> but I'm not sure if it is necessary. LKRG has 10+ configurable options and this 
> list will probably expand over the time. I'm not sure if we should add all of 
> them as a possible module's argument. Unless you are thinking only about hiding 
> feature itself as a 2nd argument. Nevertheless, it is still an option to 
> rebuild LKRG and change default hiding option in the source code to be 1 
> instead of 0 and you should get the same effect.

Ok, I understand, but what I was talking about is that changing default value
won't work unless we add this:

--- p_lkrg_main.c_orig  2019-09-25 13:33:36.665971509 +0300
+++ p_lkrg_main.c       2019-09-25 13:33:46.185966967 +0300
@@ -160,6 +160,7 @@
 #endif
 
    if (p_lkrg_global_ctrl.p_hide_module) {
+      p_lkrg_global_ctrl.p_hide_module = 0;
       p_hide_itself();
    }


> On Thu, Sep 19, 2019 at 12:38:53PM +0300, Alexander V. Gusev wrote:
> > On Wed, Sep 18, 2019 at 05:31:05PM +0200, Adam Zabrocki wrote:
> > 
> > > I've reviewed your patch and I do agree it is a right approach. However, I have 
> > > in my TODO list to completely change / rewrite coding style. It is not 
> > > the highest priority right know but I'm aware this should be done at some 
> > > point. Most likely we should match Linux kernel coding style.
> > > I hope you are fine if I leave the patch how it is now since I hope it will be 
> > > completely changed.
> > 
> > Sure, I am totally fine with that!
> > 
> > > I would also be hapy for any help in that. If you have 
> > > time and energy feel free to address it and we should cooperate :)
> > 
> > Well, I do have _some_ time and energy. How exactly would you like me
> > to cooperate?
> > 
> > > > IMHO, this patch creates a potential problem if p_hide_module
> > > > once becomes a module parameter and not a constant.
> > > > ...
> > > 
> > > Good catch! We didn't plan to introduce that feature as argument
> > 
> > Why not? I am loading lkrg in initrd, as early as possible, then at some
> > point during bootup the systemd unit starts and continues setting up
> > the module via sysctl. In this case it is more convenient to immediately
> > adjust all settings through module parameters, and it is probably more
> > secure.
> > 
> > > but certainly 
> > > this lock should be removed from the main call. I've just push the fix 
> > > (including lock removal) here:
> > > 
> > > https://bitbucket.org/Adam_pi3/lkrg-main/commits/97c20439c9414ad7a3b22dd76888967f36634a77
> > 
> > Great, thank you!
> > 
> > > > On Tue, Sep 17, 2019 at 06:04:46PM +0200, Adam Zabrocki wrote:
> > > > > Hi,
> > > > > 
> > > > > I've tested attached patch and it should works fine. Nevertheless, before 
> > > > > merging it into the official repo I would be thankful if you could make some 
> > > > > tests on your side as well.
> > > > > 
> > > > > Best regards,
> > > > > Adam
> > > > > 
> > > > > On Tue, Sep 17, 2019 at 12:37:23AM +0200, Adam Zabrocki wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > Thanks for the report.
> > > > > > 
> > > > > > On Mon, Sep 16, 2019 at 02:09:19PM +0300, Alexander V. Gusev wrote:
> > > > > > > Hi!
> > > > > > > 
> > > > > > > I am having trouble with sysctl lkrg.hide. lkrg-0.7 is
> > > > > > > compiled from source on Debian 10 (4.19.37-5). I insmod
> > > > > > > p_lkrg.ko, then do:
> > > > > > > 
> > > > > > >   sysctl lrkg.hide=1
> > > > > > > 
> > > > > > > In a few seconds lkrg starts complaining that a module has
> > > > > > > been lost and is missing from module list. Looks like lkrg
> > > > > > > forgot that it's himself that is missing because it just
> > > > > > > did hide itself!
> > > > > > > 
> > > > > > > Perhaps I am missing something or doing something wrong?
> > > > > > > 
> > > > > > 
> > > > > > Nope, you are doing everything right. I've not put too much attention to this 
> > > > > > functionality. 0.7 re-enabled self-hashing feature but I miss 'hide' path and 
> > > > > > this is the reason of that issue. I will try to address this issue and let you 
> > > > > > know about the patch.
> > > > > > 
> > > > > > Thanks,
> > > > > > Adam
> > > > > > 
> > > > > > > dmesg output is as follows:
> > > > > > > 
> > > > > > > [p_lkrg] ALERT !!! FOUND LESS[1] MODULES IN CURRENT SYSTEM IN 
> > > > > > >   MODULE LIST[63] THAN IN DB MODULE LIST[64]
> > > > > > > [p_lkrg] LOST MODULE:
> > > > > > > module at addr[000000002061e0af] module core[000000008fbf4b8b] 
> > > > > > >   with size[0x11000] hash[0xd71b46568b40ba8f]
> > > > > > > [p_lkrg] ** STRANGE BEHAVIOUR DETECTED - MODULE FOUND IN DB BUT 
> > > > > > >   NOT IN OS !! **
> > > > > > >               ** RACE CONDITION MIGHT APPEARED WHEN SYSTEM WAS 
> > > > > > >               REBUILDING DATABASE **
> > > > > > > [p_lkrg] ALERT !!! FOUND LESS[1] MODULES IN CURRENT SYSTEM IN 
> > > > > > > KOBJ[63] THAN IN DB KOBJ[64]
> > > > > > > [p_lkrg] LOST MODULE:
> > > > > > > module at addr[000000002061e0af] module core[000000008fbf4b8b] 
> > > > > > > with size[0x11000] hash[0xd71b46568b40ba8f]
> > > > > > > [p_lkrg] ** STRANGE BEHAVIOUR DETECTED - MODULE FOUND IN DB BUT 
> > > > > > > NOT IN OS !! **
> > > > > > >               ** RACE CONDITION MIGHT APPEARED WHEN SYSTEM WAS 
> > > > > > >               REBUILDING DATABASE **
> > > > > > > [p_lkrg] System is clean!
> > > > > > > 
> > > > > > > Thanks!
> > > > > > 
> > > > > > -- 
> > > > > > pi3 (pi3ki31ny) - pi3 (at) itsec pl
> > > > > > http://pi3.com.pl
> > > > > > 
> > > > > 
> > > > > -- 
> > > > > pi3 (pi3ki31ny) - pi3 (at) itsec pl
> > > > > http://pi3.com.pl
> > > > > 
> > > > 
> > > > > diff --git a/src/modules/self-defense/hiding/p_hiding.c b/src/modules/self-defense/hiding/p_hiding.c
> > > > > index 53882ce..b74995e 100644
> > > > > --- a/src/modules/self-defense/hiding/p_hiding.c
> > > > > +++ b/src/modules/self-defense/hiding/p_hiding.c
> > > > > @@ -42,6 +42,10 @@ void p_hide_itself(void) {
> > > > >     p_find_sect_attrs  = p_find_me->sect_attrs;
> > > > >     p_find_notes_attrs = p_find_me->notes_attrs;
> > > > >  */
> > > > > +   p_text_section_lock();
> > > > > +   /* We are heavily consuming module list here - take 'module_mutex' */
> > > > > +   mutex_lock(&module_mutex);
> > > > > +   spin_lock(&p_db_lock);
> > > > >  
> > > > >     P_HIDE_FROM_MODULE_LIST(p_find_me);
> > > > >     P_HIDE_FROM_KOBJ(p_find_me);
> > > > > @@ -49,8 +53,26 @@ void p_hide_itself(void) {
> > > > >     P_HIDE_FROM_DDEBUG(p_find_me);
> > > > >  #endif
> > > > >  
> > > > > +   /* OK, now recalculate hashes again! */
> > > > > +   while(p_kmod_hash(&p_db.p_module_list_nr,&p_db.p_module_list_array,
> > > > > +                     &p_db.p_module_kobj_nr,&p_db.p_module_kobj_array, 0x2) != P_LKRG_SUCCESS)
> > > > > +      schedule();
> > > > > +
> > > > > +   /* Update global module list/kobj hash */
> > > > > +   p_db.p_module_list_hash = p_lkrg_fast_hash((unsigned char *)p_db.p_module_list_array,
> > > > > +                                          (unsigned int)p_db.p_module_list_nr * sizeof(p_module_list_mem));
> > > > > +
> > > > > +   p_db.p_module_kobj_hash = p_lkrg_fast_hash((unsigned char *)p_db.p_module_kobj_array,
> > > > > +                                          (unsigned int)p_db.p_module_kobj_nr * sizeof(p_module_kobj_mem));
> > > > > +   /* We should be fine now! */
> > > > > +
> > > > >     p_lkrg_global_ctrl.p_hide_module = 0x1;
> > > > >  
> > > > > +   spin_unlock(&p_db_lock);
> > > > > +   /* Release the 'module_mutex' */
> > > > > +   mutex_unlock(&module_mutex);
> > > > > +   p_text_section_unlock();
> > > > > +
> > > > >  p_hide_itself_out:
> > > > >  
> > > > >  // STRONG_DEBUG
> > > > > @@ -78,14 +100,37 @@ void p_unhide_itself(void) {
> > > > >        goto p_unhide_itself_out;
> > > > >     }
> > > > >  
> > > > > +   p_text_section_lock();
> > > > > +   /* We are heavily consuming module list here - take 'module_mutex' */
> > > > > +   mutex_lock(&module_mutex);
> > > > > +   spin_lock(&p_db_lock);
> > > > > +
> > > > >     P_UNHIDE_FROM_MODULE_LIST(p_find_me,p_global_modules);
> > > > >     P_UNHIDE_FROM_KOBJ(p_find_me,p_tmp_kset,p_tmp_ktype);
> > > > >  
> > > > >  //   P_UNHIDE_FROM_KOBJ(p_find_me,p_find_kobj_parent,
> > > > >  //                      p_find_sect_attrs,p_find_notes_attrs);
> > > > >  
> > > > > +   /* OK, now recalculate hashes again! */
> > > > > +   while(p_kmod_hash(&p_db.p_module_list_nr,&p_db.p_module_list_array,
> > > > > +                     &p_db.p_module_kobj_nr,&p_db.p_module_kobj_array, 0x2) != P_LKRG_SUCCESS)
> > > > > +      schedule();
> > > > > +
> > > > > +   /* Update global module list/kobj hash */
> > > > > +   p_db.p_module_list_hash = p_lkrg_fast_hash((unsigned char *)p_db.p_module_list_array,
> > > > > +                                          (unsigned int)p_db.p_module_list_nr * sizeof(p_module_list_mem));
> > > > > +
> > > > > +   p_db.p_module_kobj_hash = p_lkrg_fast_hash((unsigned char *)p_db.p_module_kobj_array,
> > > > > +                                          (unsigned int)p_db.p_module_kobj_nr * sizeof(p_module_kobj_mem));
> > > > > +   /* We should be fine now! */
> > > > > +
> > > > >     p_lkrg_global_ctrl.p_hide_module = 0x0;
> > > > >  
> > > > > +   spin_unlock(&p_db_lock);
> > > > > +   /* Release the 'module_mutex' */
> > > > > +   mutex_unlock(&module_mutex);
> > > > > +   p_text_section_unlock();
> > > > > +
> > > > >  p_unhide_itself_out:
> > > > >  
> > > > >  // STRONG_DEBUG
> > > > 
> > > 
> > > > diff -urN lkrg-0.7_orig/src/modules/self-defense/hiding/p_hiding.c lkrg-0.7_alt/src/modules/self-defense/hiding/p_hiding.c
> > > > --- lkrg-0.7_orig/src/modules/self-defense/hiding/p_hiding.c	2019-06-18 20:47:26.000000000 +0300
> > > > +++ lkrg-0.7_alt/src/modules/self-defense/hiding/p_hiding.c	2019-09-17 19:49:34.958257713 +0300
> > > > @@ -43,14 +43,20 @@
> > > >     p_find_notes_attrs = p_find_me->notes_attrs;
> > > >  */
> > > >  
> > > > +   P_HIDE_LOCK;
> > > > +
> > > >     P_HIDE_FROM_MODULE_LIST(p_find_me);
> > > >     P_HIDE_FROM_KOBJ(p_find_me);
> > > >  #if defined(CONFIG_DYNAMIC_DEBUG)
> > > >     P_HIDE_FROM_DDEBUG(p_find_me);
> > > >  #endif
> > > >  
> > > > +   P_HIDE_RECALC;
> > > > +
> > > >     p_lkrg_global_ctrl.p_hide_module = 0x1;
> > > >  
> > > > +   P_HIDE_UNLOCK;
> > > > +
> > > >  p_hide_itself_out:
> > > >  
> > > >  // STRONG_DEBUG
> > > > @@ -78,14 +84,20 @@
> > > >        goto p_unhide_itself_out;
> > > >     }
> > > >  
> > > > +   P_HIDE_LOCK;
> > > > +
> > > >     P_UNHIDE_FROM_MODULE_LIST(p_find_me,p_global_modules);
> > > >     P_UNHIDE_FROM_KOBJ(p_find_me,p_tmp_kset,p_tmp_ktype);
> > > >  
> > > >  //   P_UNHIDE_FROM_KOBJ(p_find_me,p_find_kobj_parent,
> > > >  //                      p_find_sect_attrs,p_find_notes_attrs);
> > > >  
> > > > +   P_HIDE_RECALC;
> > > > +
> > > >     p_lkrg_global_ctrl.p_hide_module = 0x0;
> > > >  
> > > > +   P_HIDE_UNLOCK;
> > > > +
> > > >  p_unhide_itself_out:
> > > >  
> > > >  // STRONG_DEBUG
> > > > diff -urN lkrg-0.7_orig/src/modules/self-defense/hiding/p_hiding.h lkrg-0.7_alt/src/modules/self-defense/hiding/p_hiding.h
> > > > --- lkrg-0.7_orig/src/modules/self-defense/hiding/p_hiding.h	2019-06-18 20:47:26.000000000 +0300
> > > > +++ lkrg-0.7_alt/src/modules/self-defense/hiding/p_hiding.h	2019-09-17 19:46:21.842349857 +0300
> > > > @@ -135,6 +135,30 @@
> > > >  */
> > > >  #endif
> > > >  
> > > > +#define P_HIDE_LOCK \
> > > > +   p_text_section_lock(); \
> > > > +   /* We are heavily consuming module list here - take 'module_mutex' */ \
> > > > +   mutex_lock(&module_mutex); \
> > > > +   spin_lock(&p_db_lock); \
> > > > +
> > > > +#define P_HIDE_UNLOCK \
> > > > +   spin_unlock(&p_db_lock); \
> > > > +   /* Release the 'module_mutex' */ \
> > > > +   mutex_unlock(&module_mutex); \
> > > > +   p_text_section_unlock();
> > > > +
> > > > +#define P_HIDE_RECALC \
> > > > +   /* OK, now recalculate hashes again! */ \
> > > > +   while(p_kmod_hash(&p_db.p_module_list_nr,&p_db.p_module_list_array, \
> > > > +                     &p_db.p_module_kobj_nr,&p_db.p_module_kobj_array, 0x2) != P_LKRG_SUCCESS) \
> > > > +      schedule(); \
> > > > +   /* Update global module list/kobj hash */ \
> > > > +   p_db.p_module_list_hash = p_lkrg_fast_hash((unsigned char *)p_db.p_module_list_array, \
> > > > +                                          (unsigned int)p_db.p_module_list_nr * sizeof(p_module_list_mem)); \
> > > > +   p_db.p_module_kobj_hash = p_lkrg_fast_hash((unsigned char *)p_db.p_module_kobj_array, \
> > > > +                                          (unsigned int)p_db.p_module_kobj_nr * sizeof(p_module_kobj_mem)); \
> > > > +   /* We should be fine now! */ 
> > > > +
> > > >  
> > > >  extern struct module *p_find_me;
> > > >  
> > > 
> > > 
> > > -- 
> > > pi3 (pi3ki31ny) - pi3 (at) itsec pl
> > > http://pi3.com.pl
> > > 
> 
> -- 
> pi3 (pi3ki31ny) - pi3 (at) itsec pl
> http://pi3.com.pl
> 

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.