lkdtm: do not depend on CONFIG_BLOCK
diff mbox series

Message ID ca27a073c872e243be1d17a89afb4b4aeb9a298b.1541714054.git.christophe.leroy@c-s.fr
State New
Headers show
Series
  • lkdtm: do not depend on CONFIG_BLOCK
Related show

Commit Message

Christophe Leroy Nov. 9, 2018, 7:05 a.m. UTC
Most parts of lkdtm don't require CONFIG_BLOCK.

This patch limits dependency to CONFIG_BLOCK in order to give embedded
platforms which don't select CONFIG_BLOCK the opportunity to use LKDTM.

Fixes: fddd9cf82c9f ("make LKDTM depend on BLOCK")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/misc/lkdtm/core.c | 7 ++++++-
 lib/Kconfig.debug         | 1 -
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Greg Kroah-Hartman Nov. 27, 2018, 7:43 a.m. UTC | #1
On Fri, Nov 09, 2018 at 07:05:51AM +0000, Christophe Leroy wrote:
> Most parts of lkdtm don't require CONFIG_BLOCK.
> 
> This patch limits dependency to CONFIG_BLOCK in order to give embedded
> platforms which don't select CONFIG_BLOCK the opportunity to use LKDTM.
> 
> Fixes: fddd9cf82c9f ("make LKDTM depend on BLOCK")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  drivers/misc/lkdtm/core.c | 7 ++++++-
>  lib/Kconfig.debug         | 1 -
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> index 2837dc77478e..bc76756b7eda 100644
> --- a/drivers/misc/lkdtm/core.c
> +++ b/drivers/misc/lkdtm/core.c
> @@ -40,9 +40,12 @@
>  #include <linux/interrupt.h>
>  #include <linux/hrtimer.h>
>  #include <linux/slab.h>
> -#include <scsi/scsi_cmnd.h>
>  #include <linux/debugfs.h>
>  
> +#ifdef CONFIG_BLOCK
> +#include <scsi/scsi_cmnd.h>
> +#endif

Why would this config option be needed to be checked just to be able to
include a .h file?

And shouldn't you be depending on SCSI instead?

> +
>  #ifdef CONFIG_IDE
>  #include <linux/ide.h>
>  #endif
> @@ -101,7 +104,9 @@ static struct crashpoint crashpoints[] = {
>  	CRASHPOINT("FS_DEVRW",		 "ll_rw_block"),
>  	CRASHPOINT("MEM_SWAPOUT",	 "shrink_inactive_list"),
>  	CRASHPOINT("TIMERADD",		 "hrtimer_start"),
> +# ifdef CONFIG_BLOCK
>  	CRASHPOINT("SCSI_DISPATCH_CMD",	 "scsi_dispatch_cmd"),
> +# endif

Again, scsi?

thanks,

greg k-h
Christophe Leroy Nov. 27, 2018, 3:59 p.m. UTC | #2
On 11/27/2018 07:43 AM, Greg Kroah-Hartman wrote:
> On Fri, Nov 09, 2018 at 07:05:51AM +0000, Christophe Leroy wrote:
>> Most parts of lkdtm don't require CONFIG_BLOCK.
>>
>> This patch limits dependency to CONFIG_BLOCK in order to give embedded
>> platforms which don't select CONFIG_BLOCK the opportunity to use LKDTM.
>>
>> Fixes: fddd9cf82c9f ("make LKDTM depend on BLOCK")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   drivers/misc/lkdtm/core.c | 7 ++++++-
>>   lib/Kconfig.debug         | 1 -
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
>> index 2837dc77478e..bc76756b7eda 100644
>> --- a/drivers/misc/lkdtm/core.c
>> +++ b/drivers/misc/lkdtm/core.c
>> @@ -40,9 +40,12 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/hrtimer.h>
>>   #include <linux/slab.h>
>> -#include <scsi/scsi_cmnd.h>
>>   #include <linux/debugfs.h>
>>   
>> +#ifdef CONFIG_BLOCK
>> +#include <scsi/scsi_cmnd.h>
>> +#endif
> 
> Why would this config option be needed to be checked just to be able to
> include a .h file?

Because if including that file regardless of the config option, you get:

   CALL    scripts/checksyscalls.sh
   CC      drivers/misc/lkdtm/core.o
In file included from ./include/scsi/scsi_cmnd.h:7:0,
                  from drivers/misc/lkdtm/core.c:45:
./include/linux/t10-pi.h:40:41: warning: 'struct request' declared 
inside parameter list
  static inline u32 t10_pi_ref_tag(struct request *rq)
                                          ^
./include/linux/t10-pi.h:40:41: warning: its scope is only this 
definition or declaration, which is probably not what you want
./include/linux/t10-pi.h:61:8: warning: 'struct request' declared inside 
parameter list
         unsigned int intervals)
         ^
./include/linux/t10-pi.h:64:42: warning: 'struct request' declared 
inside parameter list
  static inline void t10_pi_prepare(struct request *rq, u8 protection_type)
                                           ^
In file included from ./include/scsi/scsi_cmnd.h:12:0,
                  from drivers/misc/lkdtm/core.c:45:
./include/scsi/scsi_device.h:435:4: error: unknown type name 'req_flags_t'
     req_flags_t rq_flags, int *resid);
     ^
./include/scsi/scsi_device.h: In function 'scsi_execute_req':
./include/scsi/scsi_device.h:442:2: error: implicit declaration of 
function '__scsi_execute' [-Werror=implicit-function-declaration]
   __scsi_execute(sdev, cmd, data_direction, buffer, bufflen, \
   ^
./include/scsi/scsi_device.h:451:9: note: in expansion of macro 
'scsi_execute'
   return scsi_execute(sdev, cmd, data_direction, buffer,
          ^
In file included from ./include/scsi/scsi_request.h:5:0,
                  from ./include/scsi/scsi_cmnd.h:13,
                  from drivers/misc/lkdtm/core.c:45:
./include/linux/blk-mq.h: At top level:
./include/linux/blk-mq.h:100:9: error: type defaults to 'int' in 
declaration of 'blk_status_t' [-Werror=implicit-int]
  typedef blk_status_t (queue_rq_fn)(struct blk_mq_hw_ctx *,
          ^
./include/linux/blk-mq.h:100:9: error: 'blk_status_t' declared as 
function returning a function
./include/linux/blk-mq.h:101:16: warning: parameter names (without 
types) in function declaration
    const struct blk_mq_queue_data *);
                 ^
./include/linux/blk-mq.h:123:2: error: unknown type name 'queue_rq_fn'
   queue_rq_fn  *queue_rq;
   ^
./include/linux/blk-mq.h:144:2: error: unknown type name 'softirq_done_fn'
   softirq_done_fn  *complete;
   ^
./include/linux/blk-mq.h:223:31: error: 'blk_mq_req_flags_t' undeclared 
here (not in a function)
   BLK_MQ_REQ_NOWAIT = (__force blk_mq_req_flags_t)(1 << 0),
                                ^
./include/linux/blk-mq.h:233:3: error: expected declaration specifiers 
or '...' before 'blk_mq_req_flags_t'
    blk_mq_req_flags_t flags);
    ^
./include/linux/blk-mq.h:235:20: error: expected declaration specifiers 
or '...' before 'blk_mq_req_flags_t'
    unsigned int op, blk_mq_req_flags_t flags,
                     ^
In file included from ./arch/powerpc/include/asm/atomic.h:11:0,
                  from ./include/linux/atomic.h:7,
                  from ./include/linux/spinlock.h:436,
                  from ./include/linux/wait.h:9,
                  from ./include/linux/wait_bit.h:8,
                  from ./include/linux/fs.h:6,
                  from drivers/misc/lkdtm/core.c:34:
./include/linux/blk-mq.h: In function 'blk_mq_mark_complete':
./include/linux/blk-mq.h:306:20: error: dereferencing pointer to 
incomplete type 'struct request'
   return cmpxchg(&rq->state, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE) ==
                     ^
./arch/powerpc/include/asm/cmpxchg.h:483:19: note: in definition of 
macro 'cmpxchg'
       __typeof__(*(ptr)) _o_ = (o);      \
                    ^
./include/linux/blk-mq.h:306:29: error: 'MQ_RQ_IN_FLIGHT' undeclared 
(first use in this function)
   return cmpxchg(&rq->state, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE) ==
                              ^
./arch/powerpc/include/asm/cmpxchg.h:483:32: note: in definition of 
macro 'cmpxchg'
       __typeof__(*(ptr)) _o_ = (o);      \
                                 ^
./include/linux/blk-mq.h:306:29: note: each undeclared identifier is 
reported only once for each function it appears in
   return cmpxchg(&rq->state, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE) ==
                              ^
./arch/powerpc/include/asm/cmpxchg.h:483:32: note: in definition of 
macro 'cmpxchg'
       __typeof__(*(ptr)) _o_ = (o);      \
                                 ^
./include/linux/blk-mq.h:306:46: error: 'MQ_RQ_COMPLETE' undeclared 
(first use in this function)
   return cmpxchg(&rq->state, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE) ==
                                               ^
./arch/powerpc/include/asm/cmpxchg.h:484:32: note: in definition of 
macro 'cmpxchg'
       __typeof__(*(ptr)) _n_ = (n);      \
                                 ^
In file included from ./include/scsi/scsi_request.h:5:0,
                  from ./include/scsi/scsi_cmnd.h:13,
                  from drivers/misc/lkdtm/core.c:45:
./include/linux/blk-mq.h: In function 'blk_mq_rq_from_pdu':
./include/linux/blk-mq.h:316:22: error: invalid application of 'sizeof' 
to incomplete type 'struct request'
   return pdu - sizeof(struct request);
                       ^
./include/linux/blk-mq.h: In function 'blk_mq_rq_to_pdu':
./include/linux/blk-mq.h:320:2: error: invalid use of undefined type 
'struct request'
   return rq + 1;
   ^
In file included from drivers/misc/lkdtm/core.c:45:0:
./include/scsi/scsi_cmnd.h: In function 'scsi_bidi_cmnd':
./include/scsi/scsi_cmnd.h:214:9: error: implicit declaration of 
function 'blk_bidi_rq' [-Werror=implicit-function-declaration]
   return blk_bidi_rq(cmd->request) &&
          ^
./include/scsi/scsi_cmnd.h: In function 'scsi_get_lba':
./include/scsi/scsi_cmnd.h:307:9: error: implicit declaration of 
function 'blk_rq_pos' [-Werror=implicit-function-declaration]
   return blk_rq_pos(scmd->request);
          ^
cc1: some warnings being treated as errors
make[1]: *** [drivers/misc/lkdtm/core.o] Error 1
make: *** [drivers/misc/lkdtm/core.o] Error 2


> 
> And shouldn't you be depending on SCSI instead?

Yes maybe. I did make it depend on CONFIG_BLOCK because it was what it 
was depending on in the Kconfig. I'll send a v2

Christophe

> 
>> +
>>   #ifdef CONFIG_IDE
>>   #include <linux/ide.h>
>>   #endif
>> @@ -101,7 +104,9 @@ static struct crashpoint crashpoints[] = {
>>   	CRASHPOINT("FS_DEVRW",		 "ll_rw_block"),
>>   	CRASHPOINT("MEM_SWAPOUT",	 "shrink_inactive_list"),
>>   	CRASHPOINT("TIMERADD",		 "hrtimer_start"),
>> +# ifdef CONFIG_BLOCK
>>   	CRASHPOINT("SCSI_DISPATCH_CMD",	 "scsi_dispatch_cmd"),
>> +# endif
> 
> Again, scsi?
> 
> thanks,
> 
> greg k-h
>
Kees Cook Nov. 27, 2018, 5:03 p.m. UTC | #3
On Tue, Nov 27, 2018 at 7:59 AM, Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
> On 11/27/2018 07:43 AM, Greg Kroah-Hartman wrote:
>>
>> On Fri, Nov 09, 2018 at 07:05:51AM +0000, Christophe Leroy wrote:
>>>
>>> Most parts of lkdtm don't require CONFIG_BLOCK.
>>>
>>> This patch limits dependency to CONFIG_BLOCK in order to give embedded
>>> platforms which don't select CONFIG_BLOCK the opportunity to use LKDTM.
>>>
>>> Fixes: fddd9cf82c9f ("make LKDTM depend on BLOCK")
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Actually, I don't think any of this is needed, actually. The switch to
kprobes from jprobes meant that the symbols are resolved at runtime
now, so there's no need for the headers at all (nor the Kconfig line).
I'll spin something and send it out...

Patch
diff mbox series

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index 2837dc77478e..bc76756b7eda 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -40,9 +40,12 @@ 
 #include <linux/interrupt.h>
 #include <linux/hrtimer.h>
 #include <linux/slab.h>
-#include <scsi/scsi_cmnd.h>
 #include <linux/debugfs.h>
 
+#ifdef CONFIG_BLOCK
+#include <scsi/scsi_cmnd.h>
+#endif
+
 #ifdef CONFIG_IDE
 #include <linux/ide.h>
 #endif
@@ -101,7 +104,9 @@  static struct crashpoint crashpoints[] = {
 	CRASHPOINT("FS_DEVRW",		 "ll_rw_block"),
 	CRASHPOINT("MEM_SWAPOUT",	 "shrink_inactive_list"),
 	CRASHPOINT("TIMERADD",		 "hrtimer_start"),
+# ifdef CONFIG_BLOCK
 	CRASHPOINT("SCSI_DISPATCH_CMD",	 "scsi_dispatch_cmd"),
+# endif
 # ifdef CONFIG_IDE
 	CRASHPOINT("IDE_CORE_CP",	 "generic_ide_ioctl"),
 # endif
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af29b8224fd..0dd65b4b2ad2 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1685,7 +1685,6 @@  if RUNTIME_TESTING_MENU
 config LKDTM
 	tristate "Linux Kernel Dump Test Tool Module"
 	depends on DEBUG_FS
-	depends on BLOCK
 	help
 	This module enables testing of the different dumping mechanisms by
 	inducing system failures at predefined crash points.