From: Andrey Okoshkin <a.okoshkin@samsung.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
pclouds@gmail.com, Ivan Arishchenko <i.arishchenk@samsung.com>
Subject: [PATCH] diff: fix lstat() error handling in diff_populate_filespec()
Date: Fri, 27 Oct 2017 12:33:25 +0300 [thread overview]
Message-ID: <72e8146b-d4ab-ad7e-6afd-9168806eeca2@samsung.com> (raw)
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
next parent reply other threads:[~2017-10-27 9:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20171027093331epcas2p1a945263c12b8ba608492693da4e3eff2@epcas2p1.samsung.com>
2017-10-27 9:33 ` Andrey Okoshkin [this message]
2017-10-27 16:35 ` [PATCH] diff: fix lstat() error handling in diff_populate_filespec() Junio C Hamano
2017-10-27 16:58 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=72e8146b-d4ab-ad7e-6afd-9168806eeca2@samsung.com \
--to=a.okoshkin@samsung.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=i.arishchenk@samsung.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox