On Wed, Apr 25, 2018 at 5:44 PM, Jason Dillaman <jdillama@xxxxxxxxxx> wrote: > On Wed, Apr 25, 2018 at 6:28 AM, Ilya Dryomov <idryomov@xxxxxxxxx> wrote: >> ceph_con_workfn() validates con->state before calling try_read() and >> then try_write(). However, try_read() temporarily releases con->mutex, >> notably in process_message() and ceph_con_in_msg_alloc(), opening the >> window for ceph_con_close() to sneak in, close the connection and >> release con->sock. When try_write() is called on the assumption that >> con->state is still valid (i.e. not STANDBY or CLOSED), a NULL sock >> gets passed to the networking stack: >> >> BUG: unable to handle kernel NULL pointer dereference at 0000000000000020 >> IP: selinux_socket_sendmsg+0x5/0x20 >> >> Make sure con->state is valid at the top of try_write() and add an >> explicit BUG_ON for this, similar to try_read(). >> >> Cc: stable@xxxxxxxxxxxxxxx >> Link: https://tracker.ceph.com/issues/23706 >> Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx> >> --- >> net/ceph/messenger.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c >> index fcb40c12b1f8..a7bfc07d2876 100644 >> --- a/net/ceph/messenger.c >> +++ b/net/ceph/messenger.c >> @@ -2569,6 +2569,10 @@ static int try_write(struct ceph_connection *con) >> int ret = 1; >> >> dout("try_write start %p state %lu\n", con, con->state); >> + if (con->state != CON_STATE_PREOPEN && >> + con->state != CON_STATE_NEGOTIATING && >> + con->state != CON_STATE_OPEN) >> + return 0; >> >> more: >> dout("try_write out_kvec_bytes %d\n", con->out_kvec_bytes); >> @@ -2594,6 +2598,8 @@ static int try_write(struct ceph_connection *con) >> } >> >> more_kvec: >> + BUG_ON(!con->sock); >> + >> /* kvec data queued? */ >> if (con->out_kvec_left) { >> ret = write_partial_kvec(con); >> -- >> 2.4.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Reviewed-by: Jason Dillaman <dillaman@xxxxxxxxxx> Actually, even though try_write() is where PREOPEN -> CONNECTING transition happens, it is technically possible for try_write() to be called in CONNECTING. Denying it shouldn't break anything, but no point in being overly restrictive here. I have updated the patch: diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index a7bfc07d2876..3b3d33ea9ed8 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -2570,6 +2570,7 @@ static int try_write(struct ceph_connection *con) dout("try_write start %p state %lu\n", con, con->state); if (con->state != CON_STATE_PREOPEN && + con->state != CON_STATE_CONNECTING && con->state != CON_STATE_NEGOTIATING && con->state != CON_STATE_OPEN) return 0; Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
