[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190918153105.GA10740@pi3.com.pl>
Date: Wed, 18 Sep 2019 17:31:05 +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,
On Tue, Sep 17, 2019 at 08:10:10PM +0300, Alexander V. Gusev wrote:
>
> Hi, Adam!
>
> The patched version works fine for me too, thanks!
Great, thanks for testing!
>
> Please consider my somewhat reformatted version of your patch
> that avoids code duplication (attached).
>
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. I would also be happy for any help in that. If you have
time and energy feel free to address it and we should cooperate :)
> 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;
> }
>
Good catch! We didn't plan to introduce that feature as argument 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
Thanks,
Adam
> 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
>
> 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.
