Git development
 help / color / mirror / Atom feed
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

       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