[PATCH] diff: fix lstat() error handling in diff_populate_filespec()
- From
Andrey Okoshkin <a.okoshkin@samsung.com>
- Date
- Oct 27, 2017, 09:33 UTC
- Message-ID
- <72e8146b-d4ab-ad7e-6afd-9168806eeca2@samsung.com>
- In-Reply-To
- <CGME20171027093331epcas2p1a945263c12b8ba608492693da4e3eff2@epcas2p1.samsung.com>
Add lstat() error handling not only for ENOENT case. Otherwise uninitialised 'struct stat st' variable is used later in case of lstat() non-ENOENT failure which leads to processing of rubbish values of file mode ('S_ISLNK(st.st_mode)' check) or size ('xsize_t(st.st_size)').
Signed-off-by: Andrey Okoshkin <a.okoshkin@samsung.com> --- Hello,
I've injected a fault to git binary with the internal tool for fault tolerance evaluation: lstat() returns '-1' and errno is set to 'EACCES' at diff_populate_filespec git/diff.c:2850. In a real life it's very difficult to reproduce such behaviour. I'm not sure why only ENOENT error of lstat() is considered as an error but passing by other errno values leads to reading of uninitialized 'struct stat st' variable. It means that the populated 'diff_filespec' structure may be incorrectly filled.
Also diff_populate_filespec() result is not checked at diff_filespec_is_binary() but it seems OK there.
Best regards, Andrey
diff.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/diff.c b/diff.c index 8406a8324..d737a78a1 100644 --- a/diff.c +++ b/diff.c @@ -2848,14 +2848,12 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags) int fd; if (lstat(s->path, &st) < 0) { - if (errno == ENOENT) { - err_empty: - err = -1; - empty: - s->data = (char *)""; - s->size = 0; - return err; - } + err_empty: + err = -1; + empty: + s->data = (char *)""; + s->size = 0; + return err; } s->size = xsize_t(st.st_size); if (!s->size)
-- 2.14.3