Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190917171010.p7kde6vb7h7d33vl@gusev>
Date: Tue, 17 Sep 2019 20:10:10 +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


Hi, Adam!

The patched version works fine for me too, thanks!

Please consider my somewhat reformatted version of your patch
that avoids code duplication (attached).

IMHO, this patch creates a potential problem if p_hide_module
once becomes a module parameter and not a constant. p_hide_itself
is also called in p_lkrg_main.c:

   mutex_lock(&module_mutex);
   if (p_lkrg_global_ctrl.p_hide_module) {
      p_hide_itself();
   }
   mutex_unlock(&module_mutex);

This creates a possibility for mutex_lock(&module_mutex) to be
called twice, both outside and inside of the function.

Yet, at the moment this code in p_lkrg_main.c appears to be dead:
p_hide_module at this point is always 0, and inside p_hide_itself()
hiding will be refused if it is non-zero:

   if (p_lkrg_global_ctrl.p_hide_module) {
   ...
      goto p_hide_itself_out;
   }

Thanks!

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


View attachment "alt_hide_patch.diff" of type "text/x-diff" (2636 bytes)

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.