On Fri, 15 Sept 2023 at 18:02, Viktor Prutyanov <[email protected]> wrote:
>
> PE export name check introduced in d399d6b179 isn't reliable enough,
> because a page with the export directory may be not present for some
> reason. On the other hand, elf2dmp retrieves the PDB name in any case.
> It can be also used to check that a PE image is the kernel image. So,
> check PDB name when searching for Windows kernel image.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2165917
>
> Signed-off-by: Viktor Prutyanov <[email protected]>
> ---
> contrib/elf2dmp/main.c | 93 +++++++++++++++---------------------------
> 1 file changed, 33 insertions(+), 60 deletions(-)
Hi; Coverity points out a bug in this code (CID 1521598):
> -static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
> - char *hash, struct va_space *vs)
> +static bool pe_check_pdb_name(uint64_t base, void *start_addr,
> + struct va_space *vs, OMFSignatureRSDS *rsds)
> {
> const char sign_rsds[4] = "RSDS";
sign_rsds[] is not NUL-terminated...
> IMAGE_DEBUG_DIRECTORY debug_dir;
> - OMFSignatureRSDS rsds;
> - char *pdb_name;
> - size_t pdb_name_sz;
> - size_t i;
> + char pdb_name[sizeof(PDB_NAME)];
>
> if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_DEBUG_DIRECTORY,
> &debug_dir, sizeof(debug_dir), vs)) {
> eprintf("Failed to get Debug Directory\n");
> - return 1;
> + return false;
> }
>
> if (debug_dir.Type != IMAGE_DEBUG_TYPE_CODEVIEW) {
> - return 1;
> + eprintf("Debug Directory type is not CodeView\n");
> + return false;
> }
>
> if (va_space_rw(vs,
> base + debug_dir.AddressOfRawData,
> - &rsds, sizeof(rsds), 0)) {
> - return 1;
> + rsds, sizeof(*rsds), 0)) {
> + eprintf("Failed to resolve OMFSignatureRSDS\n");
> + return false;
> }
>
> - printf("CodeView signature is \'%.4s\'\n", rsds.Signature);
> -
> - if (memcmp(&rsds.Signature, sign_rsds, sizeof(sign_rsds))) {
> - return 1;
> + if (memcmp(&rsds->Signature, sign_rsds, sizeof(sign_rsds))) {
> + eprintf("CodeView signature is \'%.4s\', \'%s\' expected\n",
> + rsds->Signature, sign_rsds);
...but in this printf() we pass sign_rsds to a "%s" format
specifier, which requires NUL termination.
If you want to print a non-NUL-terminated string you need
to do the same thing we do for rsds->Signature, which is
give it a precision which is the correct length so printf
doesn't read off the end (i.e. "%.4s").
thanks
-- PMM
