On Thu, Jun 7, 2012 at 6:10 AM, Alex Elder <elder@xxxxxxxxxxxxx> wrote:
> On 06/06/2012 03:03 AM, Yan, Zheng wrote:
>> From: "Yan, Zheng" <zheng.z.yan@xxxxxxxxx>
>>
>> The bug can cause NULL pointer dereference in write_partial_msg_pages
>>
>> Signed-off-by: Zheng Yan <zheng.z.yan@xxxxxxxxx>
>> ---
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
