From: Junio C Hamano <junkio@cox.net>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexander Litvinov <litvinov2004@gmail.com>,
Git Mailing List <git@vger.kernel.org>
Subject: [PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout
Date: Thu, 22 Mar 2007 13:31:49 -0700 [thread overview]
Message-ID: <7virctt3yi.fsf_-_@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: <Pine.LNX.4.64.0703220931120.6730@woody.linux-foundation.org> (Linus Torvalds's message of "Thu, 22 Mar 2007 09:48:30 -0700 (PDT)")
When we write out the result of patch application, we sometimes
need to munge the data (e.g. under core.autocrlf). After doing
so, what we should free is the temporary buffer that holds the
converted data returned from convert_to_working_tree(), not the
original one.
This patch also moves the call to open() up in the function, as
the caller expects us to fail cheaply if leading directories
need to be created (and then the caller creates them and calls
us again). For that calling pattern, attempting conversion
before opening the file adds unnecessary overhead.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
Linus Torvalds <torvalds@linux-foundation.org> writes:
> In "try_create_file()", we do:
>
> ...
> if (convert_to_working_tree(path, &nbuf, &nsize)) {
> free((char *) buf);
> buf = nbuf;
> size = nsize;
> }
> ...
>
> but the thing is, the *caller* still uses the old "buf/nsize", so when you
> free it, the caller will now use the free'd data structure, and if it gets
> re-used by - for example - the zlib deflate() buffers, you'll get a
> corrupt object (if it gets re-used *before*, you'll get the *wrong*
> object!). Exactly Alexander's patterns.
>
> Alexander - sorry for all the trouble, this was definitely our bad.
I concur. Sorry for this, Alexander.
git-apply in general is quite leaky (e.g. it never frees a
finished patch, and freeing a patch is quite difficult as some
strings like filenames are shared without refcounting), but
this part deals with a large amount of data and I would rather
not add a new one.
builtin-apply.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index dfa1716..27a182b 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2355,7 +2355,7 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)
{
- int fd;
+ int fd, converted;
char *nbuf;
unsigned long nsize;
@@ -2364,17 +2364,18 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
* terminated.
*/
return symlink(buf, path);
+
+ fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);
+ if (fd < 0)
+ return -1;
+
nsize = size;
nbuf = (char *) buf;
- if (convert_to_working_tree(path, &nbuf, &nsize)) {
- free((char *) buf);
+ converted = convert_to_working_tree(path, &nbuf, &nsize);
+ if (converted) {
buf = nbuf;
size = nsize;
}
-
- fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);
- if (fd < 0)
- return -1;
while (size) {
int written = xwrite(fd, buf, size);
if (written < 0)
@@ -2386,6 +2387,8 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
}
if (close(fd) < 0)
die("closing file %s: %s", path, strerror(errno));
+ if (converted)
+ free(nbuf);
return 0;
}
next prev parent reply other threads:[~2007-03-22 20:32 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-02-28 4:36 My git repo is broken, how to fix it ? Alexander Litvinov
2007-02-28 4:57 ` Linus Torvalds
2007-02-28 11:54 ` Alexander Litvinov
2007-02-28 16:19 ` Linus Torvalds
2007-02-28 19:12 ` Alex Riesen
2007-03-19 13:32 ` Alexander Litvinov
2007-03-19 15:20 ` Linus Torvalds
[not found] ` <200703201013.39169.litvinov2004@gmail.com>
2007-03-20 5:34 ` Linus Torvalds
2007-03-20 6:55 ` Alexander Litvinov
2007-03-20 7:42 ` Junio C Hamano
2007-03-20 15:23 ` Nicolas Pitre
[not found] ` <Pine.LNX.4.64.0703200832150.6730@woody.linux-foundation.org>
[not found] ` <Pine.LNX.4.64.0703200836490.6730@woody.linux-foundation.org>
[not found] ` <200703210956.50018.litvinov2004@gmail.com>
2007-03-22 15:58 ` Linus Torvalds
2007-03-22 16:34 ` Nicolas Pitre
[not found] ` <200703211024.04740.litvinov2004@gmail.com>
2007-03-22 16:17 ` Linus Torvalds
2007-03-22 16:29 ` Linus Torvalds
2007-03-22 16:48 ` Linus Torvalds
2007-03-22 17:01 ` Nicolas Pitre
2007-03-22 17:10 ` Linus Torvalds
2007-03-22 17:28 ` Nicolas Pitre
2007-03-22 22:13 ` Jeff King
2007-03-23 0:25 ` Linus Torvalds
2007-03-23 0:42 ` Bill Lear
2007-03-23 0:51 ` Jeff King
2007-03-22 20:31 ` Junio C Hamano [this message]
2007-03-22 20:55 ` [PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout Linus Torvalds
2007-03-23 3:55 ` Alexander Litvinov
2007-03-23 3:40 ` My git repo is broken, how to fix it ? Alexander Litvinov
2007-03-22 17:12 ` Johannes Sixt
-- strict thread matches above, loose matches on Subject: below --
2021-01-29 6:29 [PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout 皐月
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=7virctt3yi.fsf_-_@assigned-by-dhcp.cox.net \
--to=junkio@cox.net \
--cc=git@vger.kernel.org \
--cc=litvinov2004@gmail.com \
--cc=torvalds@linux-foundation.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).