Re: segfault in git-remote-http

2013-04-10 Thread Jeff King
On Wed, Apr 10, 2013 at 12:16:25PM -0700, rh wrote:

> > > This returns no error on the command line and produced the segfault
> > > reported by the kernel. git clone returns immediately.
> > 
> > It does correctly report a failed exit code. The lack of message is
> > because git assumes that the helper will produce a useful message
> > before dying, but obviously it doesn't.  There's already a patch[1]
> > to fix this, but it hasn't been merged yet.
> 
> Oh yeah, I didn't check $? but I got sidetracked seeing the segfault.

I forgot to link to the patch, but it's:

  http://article.gmane.org/gmane.comp.version-control.git/220427

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: segfault in git-remote-http

2013-04-10 Thread Jeff King
On Wed, Apr 10, 2013 at 02:51:14PM -0400, Jeff King wrote:

> As for why dmesg reports git-remote-http, I'm not sure. If you "strace
> -f" the command, you can see that git is running git-remote-https. Why
> the kernel chooses to report "git-remote-http", I don't know; you'd have
> to look into how the kernel makes that decision. But I doubt it is
> related to the reason for the segfault in the first place.

Ah, I see. The hard links are a red herring. The kernel's message uses
task->comm, which is presumably set by truncating the basename of the
program to 15 characters (16 bytes with a trailing NUL).

  3.6 /proc//comm  & /proc//task//comm
  
  These files provide a method to access a tasks comm value. It also
  allows for a task to set its own or one of its thread siblings comm
  value. The comm value is limited in size compared to the cmdline
  value, so writing anything longer then the kernel's TASK_COMM_LEN
  (currently 16 chars) will result in a truncated comm value.

Try:

  $ echo 'int main() { sleep(5); *(int *)0=0; }' >foo.c
  $ gcc -o 12345678901234567890 foo.c
  $ ./123* &
  $ cat /proc/$!/comm
  123456789012345
  $ sleep 5; dmesg | tail -n 1
  [2602639.353584] 123456789012345[23062]: segfault at 0 ip 00400524 sp 
7fff46bb0700 error 6 in 12345678901234567890[40+1000]

In both cases we only get the first 15 bytes of the program name. And
indeed, "git-remote-http" is exactly 15 bytes. So it is dumb luck that
the limit is such that truncating the name makes it look like another
program.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: segfault in git-remote-http

2013-04-10 Thread Jeff King
On Wed, Apr 10, 2013 at 09:08:50AM -0700, rh wrote:

> > which should show both program names. Git invokes git-remote-* based
> > on the URL you fed it. So if you are seeing a segfault in
> > git-remote-http, presumably you fed it an http URL (which may still
> > execute SSL code if it redirects to an https URL).
> 
> Here's the command I ran (from initial post):
> git clone https://github.com/bitcoin/bitcoin.git
> 
> This returns no error on the command line and produced the segfault
> reported by the kernel. git clone returns immediately.

It does correctly report a failed exit code. The lack of message is
because git assumes that the helper will produce a useful message before
dying, but obviously it doesn't.  There's already a patch[1] to fix this,
but it hasn't been merged yet.

As for why dmesg reports git-remote-http, I'm not sure. If you "strace
-f" the command, you can see that git is running git-remote-https. Why
the kernel chooses to report "git-remote-http", I don't know; you'd have
to look into how the kernel makes that decision. But I doubt it is
related to the reason for the segfault in the first place.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: segfault in git-remote-http

2013-04-09 Thread Jeff King
On Tue, Apr 09, 2013 at 12:40:44PM -0700, rh wrote:

> > does not support hardlinks or symlinks). But I'm not sure which error
> > you are talking about. We can figure out inside the program which
> > program was invoked by checking argv, but I do not see us printing
> > remote-http anywhere.
> 
> I wasn't clear in my initial reportand may have omitted a
> significant fact. The "git clone" returned right away and I saw no
> error. The error shows up in dmesg via syslog, something like
> git-remote-http[1234]: segfault at  blah blah in libcrypto

That message is not generated by git, but rather by the kernel, which
pulls the name from argv[0], presumably. E.g., try:

  echo 'int main() { *(int *)0=0; }' >foo.c
  gcc foo.c
  ln a.out alternate
  ./a.out; ./alternate
  dmesg | tail -n 2

which should show both program names. Git invokes git-remote-* based on
the URL you fed it. So if you are seeing a segfault in git-remote-http,
presumably you fed it an http URL (which may still execute SSL code if
it redirects to an https URL).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: segfault in git-remote-http

2013-04-09 Thread Jeff King
On Tue, Apr 09, 2013 at 10:41:34AM -0700, rh wrote:

> > git-remote-http does not touch the openssl code itself at all. We only
> > talk to curl, which handles all of the ssl (and may even be built on
> > gnutls). So if that is the problem, then I think it may be a libcurl
> > bug, not a git one.
> 
> Thanks, I see git-remote-{http,https,ftp,ftps} are the same size.
> Minor nitpick but shouldn't the error thrown say git-remote-https?

They are all hardlinks to the same program (or copies if your platform
does not support hardlinks or symlinks). But I'm not sure which error
you are talking about. We can figure out inside the program which
program was invoked by checking argv, but I do not see us printing
remote-http anywhere.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: segfault in git-remote-http

2013-04-09 Thread Daniel Stenberg

On Tue, 9 Apr 2013, Jeff King wrote:

git-remote-http does not touch the openssl code itself at all. We only talk 
to curl, which handles all of the ssl (and may even be built on gnutls). So 
if that is the problem, then I think it may be a libcurl bug, not a git one.


... and if/when you do make it a libcurl bug, please include more details that 
includes how to repeat the problem and what versions of libcurl and OpenSSL 
you're using.


--

 / daniel.haxx.se
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: segfault in git-remote-http

2013-04-09 Thread Jeff King
On Tue, Apr 09, 2013 at 08:47:18AM -0700, rh wrote:

> The symptoms that this patch addresses look similar:
> 
> http://article.gmane.org/gmane.mail.postfix.user/217790
> 
> Quote from that thread:
> "This behavior is actually documented (SSL_set_fd() destroys
> a BIO already on the SSL handle, and creates a new BIO)."
> 
> Maybe someone used to looking at git-remote-http code can
> say anything about this.

git-remote-http does not touch the openssl code itself at all. We only
talk to curl, which handles all of the ssl (and may even be built on
gnutls). So if that is the problem, then I think it may be a libcurl
bug, not a git one.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html