Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 19 Sep 2019 12:38:53 +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 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
> 

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.