linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs()
@ 2018-05-10  5:37 Doug Oucharek
  2018-05-10 17:39 ` kbuild test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Doug Oucharek @ 2018-05-10  5:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman, devel, Oleg Drokin, Andreas Dilger, James Simmons
  Cc: Linux Kernel Mailing List, Lustre Development List

Under upstream staging commit 5a2ca43fa54f561c252c2, the list handling
code in kiblnd_handle_early_rxs() got changed to list_for_each_safe().
That protects against the current thread from deleting the current entry
it is looking at. It does not protect against another thread from deleting
the next item in the list (which the tmp variable points to). The way this
routine holds then releases a lock opens the door to other threads doing
just that.

This patch reverts this commit on this routine.

Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-9886
Signed-off-by: Doug Oucharek <dougso@me.com>
---
 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 32fa8ca..6148fbb 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -1965,13 +1965,14 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid,
 {
 	unsigned long flags;
 	struct kib_rx *rx;
-	struct kib_rx *tmp;
 
 	LASSERT(!in_interrupt());
 	LASSERT(conn->ibc_state >= IBLND_CONN_ESTABLISHED);
 
 	write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
-	list_for_each_entry_safe(rx, tmp, &conn->ibc_early_rxs, rx_list) {
+	while (!list_empty(&conn->ibc_early_rxs)) {
+		rx = list_entry(conn->ibc_early_rxs.next,
+				kib_rx_t, rx_list);
 		list_del(&rx->rx_list);
 		write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
 
-- 
1.8.3.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs()
  2018-05-10  5:37 [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs() Doug Oucharek
@ 2018-05-10 17:39 ` kbuild test robot
  2018-05-10 19:15 ` kbuild test robot
  2018-05-11  0:53 ` [lustre-devel] " NeilBrown
  2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2018-05-10 17:39 UTC (permalink / raw)
  To: Doug Oucharek
  Cc: devel, Greg Kroah-Hartman, Linux Kernel Mailing List,
	Oleg Drokin, kbuild-all, Andreas Dilger, Lustre Development List

[-- Attachment #1: Type: text/plain, Size: 12379 bytes --]

Hi Doug,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.17-rc4 next-20180510]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Doug-Oucharek/staging-lustre-o2iblnd-Fix-crash-in-kiblnd_handle_early_rxs/20180511-004953
config: i386-randconfig-x078-201818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h:38,
                    from drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:38:
   drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c: In function 'kiblnd_handle_early_rxs':
>> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1975:5: error: 'kib_rx_t' undeclared (first use in this function); did you mean 'kib_rx'?
        kib_rx_t, rx_list);
        ^
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
>> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
   drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1975:5: note: each undeclared identifier is reported only once for each function it appears in
        kib_rx_t, rx_list);
        ^
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
>> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
>> include/linux/kernel.h:962:48: error: expected expression before ')' token
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                                                   ^
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
>> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
   In file included from include/linux/list.h:9:0,
                    from include/linux/module.h:9,
                    from drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h:38,
                    from drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:38:
   include/linux/kernel.h:965:10: error: expected expression before ')' token
     ((type *)(__mptr - offsetof(type, member))); })
             ^
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
>> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
--
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd.h:38,
                    from drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:38:
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c: In function 'kiblnd_handle_early_rxs':
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:1975:5: error: 'kib_rx_t' undeclared (first use in this function); did you mean 'kib_rx'?
        kib_rx_t, rx_list);
        ^
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:1975:5: note: each undeclared identifier is reported only once for each function it appears in
        kib_rx_t, rx_list);
        ^
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
>> include/linux/kernel.h:962:48: error: expected expression before ')' token
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                                                   ^
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~
   In file included from include/linux/list.h:9:0,
                    from include/linux/module.h:9,
                    from drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd.h:38,
                    from drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:38:
   include/linux/kernel.h:965:10: error: expected expression before ')' token
     ((type *)(__mptr - offsetof(type, member))); })
             ^
   include/linux/list.h:366:2: note: in expansion of macro 'container_of'
     container_of(ptr, type, member)
     ^~~~~~~~~~~~
   drivers/staging/lustre//lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:8: note: in expansion of macro 'list_entry'
      rx = list_entry(conn->ibc_early_rxs.next,
           ^~~~~~~~~~

vim +1975 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c

  1962	
  1963	static void
  1964	kiblnd_handle_early_rxs(struct kib_conn *conn)
  1965	{
  1966		unsigned long flags;
  1967		struct kib_rx *rx;
  1968	
  1969		LASSERT(!in_interrupt());
  1970		LASSERT(conn->ibc_state >= IBLND_CONN_ESTABLISHED);
  1971	
  1972		write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
  1973		while (!list_empty(&conn->ibc_early_rxs)) {
> 1974			rx = list_entry(conn->ibc_early_rxs.next,
> 1975					kib_rx_t, rx_list);
  1976			list_del(&rx->rx_list);
  1977			write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
  1978	
  1979			kiblnd_handle_rx(rx);
  1980	
  1981			write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
  1982		}
  1983		write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
  1984	}
  1985	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31657 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs()
  2018-05-10  5:37 [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs() Doug Oucharek
  2018-05-10 17:39 ` kbuild test robot
@ 2018-05-10 19:15 ` kbuild test robot
  2018-05-11  0:53 ` [lustre-devel] " NeilBrown
  2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2018-05-10 19:15 UTC (permalink / raw)
  To: Doug Oucharek
  Cc: kbuild-all, Greg Kroah-Hartman, devel, Oleg Drokin,
	Andreas Dilger, James Simmons, Linux Kernel Mailing List,
	Lustre Development List

[-- Attachment #1: Type: text/plain, Size: 3383 bytes --]

Hi Doug,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.17-rc4 next-20180510]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Doug-Oucharek/staging-lustre-o2iblnd-Fix-crash-in-kiblnd_handle_early_rxs/20180511-004953
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c: In function 'kiblnd_handle_early_rxs':
   drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:161: error: 'kib_rx_t' undeclared (first use in this function); did you mean 'kib_rx'?
      rx = list_entry(conn->ibc_early_rxs.next,
                                                                                                                                                                    ^       
                                                                                                                                                                    kib_rx
   drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:161: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:171: error: expected expression before ')' token
      rx = list_entry(conn->ibc_early_rxs.next,
                                                                                                                                                                              ^
   drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1974:468: error: expected expression before ')' token
      rx = list_entry(conn->ibc_early_rxs.next,
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       ^

vim +1974 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c

  1962	
  1963	static void
  1964	kiblnd_handle_early_rxs(struct kib_conn *conn)
  1965	{
  1966		unsigned long flags;
  1967		struct kib_rx *rx;
  1968	
  1969		LASSERT(!in_interrupt());
  1970		LASSERT(conn->ibc_state >= IBLND_CONN_ESTABLISHED);
  1971	
  1972		write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
  1973		while (!list_empty(&conn->ibc_early_rxs)) {
> 1974			rx = list_entry(conn->ibc_early_rxs.next,
  1975					kib_rx_t, rx_list);
  1976			list_del(&rx->rx_list);
  1977			write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
  1978	
  1979			kiblnd_handle_rx(rx);
  1980	
  1981			write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
  1982		}
  1983		write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
  1984	}
  1985	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62264 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [lustre-devel] [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs()
  2018-05-10  5:37 [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs() Doug Oucharek
  2018-05-10 17:39 ` kbuild test robot
  2018-05-10 19:15 ` kbuild test robot
@ 2018-05-11  0:53 ` NeilBrown
  2018-05-11  2:30   ` Doug Oucharek
  2 siblings, 1 reply; 5+ messages in thread
From: NeilBrown @ 2018-05-11  0:53 UTC (permalink / raw)
  To: Doug Oucharek, Greg Kroah-Hartman, devel, Oleg Drokin,
	Andreas Dilger, James Simmons
  Cc: Linux Kernel Mailing List, Lustre Development List


[-- Attachment #1.1: Type: text/plain, Size: 2149 bytes --]

On Wed, May 09 2018, Doug Oucharek wrote:

> Under upstream staging commit 5a2ca43fa54f561c252c2, the list handling
> code in kiblnd_handle_early_rxs() got changed to list_for_each_safe().
> That protects against the current thread from deleting the current entry
> it is looking at. It does not protect against another thread from deleting
> the next item in the list (which the tmp variable points to). The way this
> routine holds then releases a lock opens the door to other threads doing
> just that.
>
> This patch reverts this commit on this routine.
>
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-9886
> Signed-off-by: Doug Oucharek <dougso@me.com>
> ---
>  drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> index 32fa8ca..6148fbb 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> @@ -1965,13 +1965,14 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid,
>  {
>  	unsigned long flags;
>  	struct kib_rx *rx;
> -	struct kib_rx *tmp;
>  
>  	LASSERT(!in_interrupt());
>  	LASSERT(conn->ibc_state >= IBLND_CONN_ESTABLISHED);
>  
>  	write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
> -	list_for_each_entry_safe(rx, tmp, &conn->ibc_early_rxs, rx_list) {
> +	while (!list_empty(&conn->ibc_early_rxs)) {
> +		rx = list_entry(conn->ibc_early_rxs.next,
> +				kib_rx_t, rx_list);
Should be:
                         struct kib_tx

Otherwise,
 Reviewed-by: NeilBrown <neilb@suse.com>

Those "convert lots of list_for_each" things really do need
careful review, don't they :-(

Thanks,
NeilBrown

>  		list_del(&rx->rx_list);
>  		write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
>  
> -- 
> 1.8.3.1
>
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [lustre-devel] [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs()
  2018-05-11  0:53 ` [lustre-devel] " NeilBrown
@ 2018-05-11  2:30   ` Doug Oucharek
  0 siblings, 0 replies; 5+ messages in thread
From: Doug Oucharek @ 2018-05-11  2:30 UTC (permalink / raw)
  To: NeilBrown
  Cc: Doug Oucharek, Greg Kroah-Hartman, devel, Oleg Drokin,
	Andreas Dilger, James Simmons, Linux Kernel Mailing List,
	Lustre Development List

[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]

I did a v2 of this patch already.

Changing to the safe version of the list macros is a mixed bag.

Doug

On May 10, 2018, at 5:53 PM, NeilBrown <neilb@suse.com<mailto:neilb@suse.com>> wrote:

On Wed, May 09 2018, Doug Oucharek wrote:

Under upstream staging commit 5a2ca43fa54f561c252c2, the list handling
code in kiblnd_handle_early_rxs() got changed to list_for_each_safe().
That protects against the current thread from deleting the current entry
it is looking at. It does not protect against another thread from deleting
the next item in the list (which the tmp variable points to). The way this
routine holds then releases a lock opens the door to other threads doing
just that.

This patch reverts this commit on this routine.

Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-9886
Signed-off-by: Doug Oucharek <dougso@me.com<mailto:dougso@me.com>>
---
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 32fa8ca..6148fbb 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -1965,13 +1965,14 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid,
{
unsigned long flags;
struct kib_rx *rx;
- struct kib_rx *tmp;

LASSERT(!in_interrupt());
LASSERT(conn->ibc_state >= IBLND_CONN_ESTABLISHED);

write_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
- list_for_each_entry_safe(rx, tmp, &conn->ibc_early_rxs, rx_list) {
+ while (!list_empty(&conn->ibc_early_rxs)) {
+ rx = list_entry(conn->ibc_early_rxs.next,
+ kib_rx_t, rx_list);
Should be:
                        struct kib_tx

Otherwise,
Reviewed-by: NeilBrown <neilb@suse.com<mailto:neilb@suse.com>>

Those "convert lots of list_for_each" things really do need
careful review, don't they :-(

Thanks,
NeilBrown

list_del(&rx->rx_list);
write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);

--
1.8.3.1

_______________________________________________
lustre-devel mailing list
lustre-devel@lists.lustre.org<mailto:lustre-devel@lists.lustre.org>
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
_______________________________________________
lustre-devel mailing list
lustre-devel@lists.lustre.org<mailto:lustre-devel@lists.lustre.org>
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org


[-- Attachment #2: Type: text/html, Size: 16027 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-11  2:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-10  5:37 [PATCH] staging: lustre: o2iblnd: Fix crash in kiblnd_handle_early_rxs() Doug Oucharek
2018-05-10 17:39 ` kbuild test robot
2018-05-10 19:15 ` kbuild test robot
2018-05-11  0:53 ` [lustre-devel] " NeilBrown
2018-05-11  2:30   ` Doug Oucharek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).