On Wed, 16 Nov 2022 07:03:12 GMT, Xue-Lei Andrew Fan <[email protected]> wrote:
>> Hi,
>>
>> May I have this update reviewed?
>>
>> The sprintf is deprecated in Xcode 14 because of security concerns, and the
>> use of it causing building failure. The build could pass if warnings are
>> disabled for codes that use sprintf method. For the long run, the sprintf
>> could be replaced with snprintf. This patch is trying to check if snprintf
>> could be used.
>>
>> Thanks,
>> Xuelei
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> address review comments
src/hotspot/os/bsd/attachListener_bsd.cpp line 295:
> 293: char msg[32];
> 294: int msg_len = os::snprintf(msg, sizeof(msg), "%d\n",
> ATTACH_ERROR_BADVERSION);
> 295: write_fully(s, msg, msg_len);
Assuming C99 behavior: safe but only because the buffer is large enough ("%d\n"
needs at most 12 bytes, buffer is 32). Were it to overflow, msg_len would be
larger than sizeof(msg) and we would probably end up reading beyond the message
end in write_fully. So not really better than using sprintf+strlen.
src/hotspot/os/bsd/attachListener_bsd.cpp line 415:
> 413: char msg[32];
> 414: int msg_len = os::snprintf(msg, sizeof(msg), "%d\n", result);
> 415: int rc = BsdAttachListener::write_fully(this->socket(), msg, msg_len);
same
src/hotspot/share/adlc/output_c.cpp line 217:
> 215: const PipeClassOperandForm *tmppipeopnd =
> 216: (const PipeClassOperandForm *)pipeclass->_localUsage[paramname];
> 217: templen += snprintf(&operand_stages[templen], operand_stages_size -
> templen, " stage_%s%c\n",
C99 Behavior: all these are probably safe but only because we never overstepped
the buffer in the first place, the buffer size is pre-calculated. If it is
incorrect and we have a truncation, subsequent writes will write beyond the
buffer.
src/hotspot/share/classfile/javaClasses.cpp line 2527:
> 2525:
> 2526: // Print stack trace line in buffer
> 2527: size_t buf_off = os::snprintf(buf, buf_size, "\tat %s.%s(",
> klass_name, method_name);
Here, and in subsequent uses: assuming C99 behavior of snprintf, if we
truncated in snprintf, buf_off will be > buffer size, (buf + buf_off) point
beyond the buffer, (buf_size - buf_off) will overflow and become very large.
-------------
PR: https://git.openjdk.org/jdk/pull/11115
