On Thu, Jun 7, 2012 at 6:10 AM, Alex Elder <[email protected]> wrote: > On 06/06/2012 03:03 AM, Yan, Zheng wrote: >> From: "Yan, Zheng" <[email protected]> >> >> The bug can cause NULL pointer dereference in write_partial_msg_pages >> >> Signed-off-by: Zheng Yan <[email protected]> >> --- >> net/ceph/messenger.c | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c >> index 1a80907..785b953 100644 >> --- a/net/ceph/messenger.c >> +++ b/net/ceph/messenger.c >> @@ -598,6 +598,7 @@ static void prepare_write_message(struct ceph_connection >> *con) >> le32_to_cpu(con->out_msg->footer.front_crc), >> le32_to_cpu(con->out_msg->footer.middle_crc)); >> >> + m->bio_iter = NULL; >> /* is there a data payload? */ >> if (le32_to_cpu(m->hdr.data_len) > 0) { >> /* initialize page iterator */ > > OK, now that I've looked at this a little bit I think I have a small > preference for doing this assignment earlier in that function, where > there is already special handling that is dependent on whether the > message being processed is being re-sent or not: > > /* > * only assign outgoing seq # if we haven't sent this message > * yet. if it is requeued, resend with it's original seq. > */ > if (m->needs_out_seq) { > m->hdr.seq = cpu_to_le64(++con->out_seq); > m->needs_out_seq = false; > } > > So I'd suggest it look like: > > /* > * only assign outgoing seq # if we haven't sent this message > * yet. if it is requeued, resend with it's original seq. > */ > if (m->needs_out_seq) { > m->hdr.seq = cpu_to_le64(++con->out_seq); > m->needs_out_seq = false; > } > #ifdef CONFIG_BLOCK > else { > /* Need to reinitialize the iterator on resends */ > m->bio_iter = NULL; > } > #endif > > Please let me know if you concur with my proposed modification or > whether you feel strongly we should go with your original fix. > I'm OK with it.
Thanks Yan, Zheng -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
