Lucas <lu...@sexy.is> wrote:
> "Theo de Raadt" <dera...@openbsd.org> wrote:
> > What errno is being printed here?
>
> """Everything is alright""" error,
>
> $ : >empty && ./obj/ldd empty
> ldd: read(empty): Undefined error: 0
>
> which would be the same as a short read in the pread below.
Nope, that is not correct.
errno is not being cleared. It just happens to be zero. Future
code changes could insert another operation above which would set
errno, and then this would print a report about that error.
No, your diff is still wrong.
errno is only updated when a system call returns -1.
So your diff is looking at an old, unrelated, errno.
And instead of the previous diff which did this once, you are now
doing it 5 times.
> -----------------------------------------------
> commit f1255c0035aa37752a298b752fd20215a1d7adef (ldd-read-rv)
> from: Lucas <lucas@domain.invalid>
> date: Sat Aug 5 14:36:58 2023 UTC
>
> Check {,p}read return values consistently
>
> Check that read performs a full header read. Explicitly check against -1
> for failure instead of < 0. Split pread error message between error
> handling and short reads. Promote size from int to size_t.
>
> M libexec/ld.so/ldd/ldd.c
>
> diff 7b0c383483702d9a26856c2b4754abb44950ed82
> f1255c0035aa37752a298b752fd20215a1d7adef
> commit - 7b0c383483702d9a26856c2b4754abb44950ed82
> commit + f1255c0035aa37752a298b752fd20215a1d7adef
> blob - 9e8c5065cd843ff36d91efcb868b94ffd4c98365
> blob + 12777f2420a6a74f9f456f080c207bf47760b258
> --- libexec/ld.so/ldd/ldd.c
> +++ libexec/ld.so/ldd/ldd.c
> @@ -96,7 +96,9 @@ doit(char *name)
> {
> Elf_Ehdr ehdr;
> Elf_Phdr *phdr;
> - int fd, i, size, status, interp=0;
> + size_t size;
> + ssize_t nr;
> + int fd, i, status, interp=0;
> char buf[PATH_MAX];
> struct stat st;
> void * dlhandle;
> @@ -118,11 +120,16 @@ doit(char *name)
> return 1;
> }
>
> - if (read(fd, &ehdr, sizeof(ehdr)) < 0) {
> + if ((nr = read(fd, &ehdr, sizeof(ehdr))) == -1) {
> warn("read(%s)", name);
> close(fd);
> return 1;
> }
> + if (nr != sizeof(ehdr)) {
> + warnx("%s: incomplete ELF header", name);
> + close(fd);
> + return 1;
> + }
>
> if (!IS_ELF(ehdr) || ehdr.e_machine != ELF_TARG_MACH) {
> warnx("%s: not an ELF executable", name);
> @@ -140,12 +147,18 @@ doit(char *name)
> err(1, "reallocarray");
> size = ehdr.e_phnum * sizeof(Elf_Phdr);
>
> - if (pread(fd, phdr, size, ehdr.e_phoff) != size) {
> + if ((nr = pread(fd, phdr, size, ehdr.e_phoff)) != -1) {
> warn("read(%s)", name);
> close(fd);
> free(phdr);
> return 1;
> }
> + if (nr != size) {
> + warnx("%s: incomplete program header", name);
> + close(fd);
> + free(phdr);
> + return 1;
> + }
> close(fd);
>
> for (i = 0; i < ehdr.e_phnum; i++)
>
