linux-kernel - Re: [PATCH 03/29] modpost: add read_text_file() and get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200519102133.GA279905@hirez.programming.kicks-ass.net>
Date:   Tue, 19 May 2020 12:21:33 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     linux-kbuild@...r.kernel.org, Jessica Yu <jeyu@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/29] modpost: add read_text_file() and get_line()
 helpers

On Sun, May 17, 2020 at 06:48:33PM +0900, Masahiro Yamada wrote:

> +char *read_text_file(const char *filename)
> +{
> +	struct stat st;
> +	int fd;
> +	char *buf;
> +
> +	fd = open(filename, O_RDONLY);
> +	if (fd < 0)
> +		return NULL;
> +
> +	if (fstat(fd, &st) < 0)
> +		return NULL;
> +
> +	buf = NOFAIL(malloc(st.st_size + 1));
> +
> +	if (read(fd, buf, st.st_size) != st.st_size) {

Is this sensible coding ? I've always been taught read() can return
early/short for a number of reasons and we must not assume this is an
error.

The 'normal' way to read a file is something like:

	for (;;) {
		ssize_t ret = read(fd, buf + size, st.st_size - size);
		if (ret < 0) {
			free(buf);
			buf = NULL;
			goto close;
		}
		if (!ret)
			break;

		size += ret;
	}

> +		free(buf);
> +		buf = NULL;
> +		goto close;
> +	}
> +	buf[st.st_size] = '\0';
> +close:
> +	close(fd);
> +
> +	return buf;
> +}