RFR: 8275846: read_base_archive_name() could read past the end of buffer [v2]

[Thomas Stuefe]

On Sat, 6 Nov 2021 06:53:52 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> src/hotspot/share/cds/filemap.cpp line 1086:
>> 
>>> 1084:   }
>>> 1085: 
>>> 1086:   char* read_base_archive_name() {
>> 
>> Why did you change the prototype? Seemed alright to me before, but may be a matter of taste.
>
> I think it's better to tighten up the API, so you don't need to worry about things like -- what value will be contained in *target if the function returns false?

Fair enough.

>> src/hotspot/share/cds/filemap.cpp line 1088:
>> 
>>> 1086:   char* read_base_archive_name() {
>>> 1087:     assert(_fd != -1, "Archive should be open");
>>> 1088:     size_t name_size = _header._base_archive_name_size;
>> 
>> Unrelated, but do we do sanity checking of header data? E.g. for a max reasonable length of _base_archive_name_size?
>
> I am planning to do that in a follow-on RFE (https://bugs.openjdk.java.net/browse/JDK-8276769). I will add more structural checks:
> 
> - GenericCDSFileMapHeader::_header_size must be smaller than the archive file
> - _base_archive_name_size/_base_archive_path_offset must be inside the header
> - the header must not overlap with the regions
> - the regions should not overlap with each other
> 
> Since more checks are added, I need to make sure the new checks also respect the -Xshare:auto flag (failure should not cause the VM to exit) which is the purpose of JDK-8276769.

Sounds good.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6285