[PATCH] lkdtm: do not depend on CONFIG_BLOCK

Subsystems: char and misc drivers, library code, linux kernel dump test module (lkdtm), the rest

STALE2810d

4 messages, 3 authors, 2018-11-27 · open the first message on its own page

[PATCH] lkdtm: do not depend on CONFIG_BLOCK

From: Christophe Leroy <hidden>
Date: 2018-11-09 07:05:56

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 <redacted>
---
 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
+
 #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.
-- 
2.13.3

Re: [PATCH] lkdtm: do not depend on CONFIG_BLOCK

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: 2018-11-27 07:43:21

On Fri, Nov 09, 2018 at 07:05:51AM +0000, Christophe Leroy wrote:
quoted hunk
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 <redacted>
---
 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?
quoted hunk
+
 #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

Re: [PATCH] lkdtm: do not depend on CONFIG_BLOCK

From: Christophe Leroy <hidden>
Date: 2018-11-27 15:59:19


On 11/27/2018 07:43 AM, Greg Kroah-Hartman wrote:
On Fri, Nov 09, 2018 at 07:05:51AM +0000, Christophe Leroy wrote:
quoted
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 <redacted>
---
  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
quoted
+
  #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

Re: [PATCH] lkdtm: do not depend on CONFIG_BLOCK

From: Kees Cook <hidden>
Date: 2018-11-27 17:03:29

On Tue, Nov 27, 2018 at 7:59 AM, Christophe Leroy
[off-list ref] wrote:

On 11/27/2018 07:43 AM, Greg Kroah-Hartman wrote:
quoted
On Fri, Nov 09, 2018 at 07:05:51AM +0000, Christophe Leroy wrote:
quoted
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 <redacted>
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...

-- 
Kees Cook
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help