Re: [PATCH] quickfetch(): Prevent overflow of the rev-list command line
- From
Johan Herland <johan@herland.net>
- Date
- Jul 8, 2009, 16:01 UTC
- Message-ID
- <200907081801.36901.johan@herland.net>
- In-Reply-To
- <4A54B755.4090100@viscovery.net>
On Wednesday 08 July 2009, Johannes Sixt wrote:
Show 30 quoted lines
> Johan Herland schrieb:
> > + /* If rev-list --stdin encounters an unknown commit, it
> > terminates, + * which will cause SIGPIPE in the write loop below.
> > */
>
> Under the conditions you describe here...
>
> > + signal(SIGPIPE, SIG_IGN);
>
> ... and SIGPIPE being ignored...
>
> > +
> > + err = start_command(&revlist);
> > + if (err) {
> > + error("could not run rev-list");
> > + return err;
> > + }
> >
> > - for (i = 0; argv[i]; i++)
> > - free(argv[i]);
> > - free(argv);
> > - return err;
> > + for (ref = ref_map; ref; ref = ref->next) {
> > + if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) <
> > 0 || + write_in_full(revlist.in, "\n", 1) < 0) {
> > + error("failed write to rev-list");
> > + err = errno;
>
> ... don't you get this error message with errno set to EPIPE?
> Previously, there was no error message.Indeed, you are correct. I guess the following should be added to the patch:
if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) < 0 ||
write_in_full(revlist.in, "\n", 1) < 0) {
- error("failed write to rev-list");
- err = errno;
+ if (errno != EPIPE) {
+ error("failed write to rev-list");
+ err = errno;
+ }
break;
}Maybe I need to do something to the close() call as well? What happens on close() after EPIPE?
Thanks,
...Johan
-- Johan Herland, <johan@herland.net> www.herland.net