Re: ldd: check read return value to avoid unitialized struct fields

[Lucas]

Greg Steuck <gne...@openbsd.org> wrote:
> Thanks for the patch.
> 
> I could see some value in tightening the conditions to always check
> `!= expected`. I don't see enough improvement from separating the error
> case of -1 from the incomplete read case considering the otherwise
> identical behavior.

Like this? The int -> size_t promotion can be a different commit or left
out.

-----------------------------------------------
commit 7663fd702838ae390515cb9326c2706a57a2983b (ldd-read-rv)
from: Lucas <lucas@domain.invalid>
date: Fri Aug 11 11:43:32 2023 UTC
 
 Check for a full read. Don't use warn when errno might be unmodified.
 Promote size from int to size_t.
 
 M  libexec/ld.so/ldd/ldd.c  |  5+  4-

1 file changed, 5 insertions(+), 4 deletions(-)

diff 7d242c13afd19e56cc21befac2ce5cdc1ac4992b 
7663fd702838ae390515cb9326c2706a57a2983b
commit - 7d242c13afd19e56cc21befac2ce5cdc1ac4992b
commit + 7663fd702838ae390515cb9326c2706a57a2983b
blob - 9e8c5065cd843ff36d91efcb868b94ffd4c98365
blob + 16b59a75e63bfe894922c4195aa11cd5e75802a1
--- libexec/ld.so/ldd/ldd.c
+++ libexec/ld.so/ldd/ldd.c
@@ -96,7 +96,8 @@ doit(char *name)
 {
        Elf_Ehdr ehdr;
        Elf_Phdr *phdr;
-       int fd, i, size, status, interp=0;
+       size_t size;
+       int fd, i, status, interp=0;
        char buf[PATH_MAX];
        struct stat st;
        void * dlhandle;
@@ -118,8 +119,8 @@ doit(char *name)
                return 1;
        }
 
-       if (read(fd, &ehdr, sizeof(ehdr)) < 0) {
-               warn("read(%s)", name);
+       if (read(fd, &ehdr, sizeof(ehdr)) != sizeof(ehdr)) {
+               warnx("%s: incomplete ELF header", name);
                close(fd);
                return 1;
        }
@@ -141,7 +142,7 @@ doit(char *name)
        size = ehdr.e_phnum * sizeof(Elf_Phdr);
 
        if (pread(fd, phdr, size, ehdr.e_phoff) != size) {
-               warn("read(%s)", name);
+               warnx("%s: incomplete program header", name);
                close(fd);
                free(phdr);
                return 1;