LKML Archive on lore.kernel.org
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).