Hi Jasonh,
Sorry for the mistake.
You are reading the event and not writing.
Patch approved.
Cheers.
He Zhenxing wrote:
> Alfranio Correia wrote:
>> Hi Jasonh,
>>
>> I did not understand the patch.
>> See some questions in-line.
>>
>
> The purpose of this patch is to make the semisync code safer from
> possible buffer overflow attacks.
>
>> Let's talk in the IRC.
>>
>> Cheers.
>>
>> He Zhenxing wrote:
>>> #At file:///media/sdb2/hezx/work/mysql/bzr/b52748/trunk-bugfixing/ based on
> revid:jon.hauglid@stripped
>>>
>>> 3057 He Zhenxing 2010-05-24
>>> BUG#52748 Semi-Sync ACK packet isn't check for length
>>>
>>> Check the length and use strncpy to make the code safer.
>>>
>>> modified:
>>> plugin/semisync/semisync_master.cc
>>> === modified file 'plugin/semisync/semisync_master.cc'
>>> --- a/plugin/semisync/semisync_master.cc 2010-03-11 02:22:18 +0000
>>> +++ b/plugin/semisync/semisync_master.cc 2010-05-24 02:57:37 +0000
>>> @@ -1048,6 +1048,7 @@ int ReplSemiSyncMaster::readSlaveReply(N
>>> const unsigned char *packet;
>>> char log_file_name[FN_REFLEN];
>>> my_off_t log_file_pos;
>>> + ulong log_file_len = 0;
>>> ulong packet_len;
>>> int result = -1;
>>>
>>> @@ -1123,7 +1124,13 @@ int ReplSemiSyncMaster::readSlaveReply(N
>>> }
>>>
>>> log_file_pos = uint8korr(packet + REPLY_BINLOG_POS_OFFSET);
>>> - strcpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET);
>>> + log_file_len = packet_len - REPLY_BINLOG_NAME_OFFSET;
>> Is this the reserved space in the packet for the log file name?
>>
>
> REPLY_BINLOG_NAME_OFFSET is the start position for the log file name in
> the reply packet, the log file name will span to the end of the packet,
> so the length of the log file will be:
> packet_len - REPLY_BINLOG_NAME_OFFSET
>
>>> + if (log_file_len > FN_REFLEN)
>>> + {
>>> + sql_print_error("Read semi-sync reply binlog file length too large");
>>> + goto l_end;
>>> + }
>> I think the "if" should be:
>>
>> if (FN_REFLEN > log_file_len)
>>
>
> No, here the code checks the sanity of the reply packet, if the length
> of log file in the reply packet is longer than FN_REFLEN, then we
> considered the reply packet is forged or corrupted, and will be
> rejected.
>
>>> + strncpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET,
> log_file_len);
>>>
>>> if (trc_level & kTraceDetail)
>>> sql_print_information("%s: Got reply (%s, %lu)",
>>>
>
>
