Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 24 Sep 2019 03:57:21 +0200
From: Adam Zabrocki <pi3@....com.pl>
To: lkrg-users@...ts.openwall.com
Subject: Re: lkrg-0.7 lkrg.hide=1 causes LOST MODULE

Hi,

Sorry for delay in contact.

> 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?
> 

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.

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.

> > > 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.
> 

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.

Thanks,
Adam


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.