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