threads / discuss / 33449

Re: segfault in git-remote-http

Subject: Re: segfault in git-remote-http

## tl;dr

17 messages between Apr 7, 2013 and Apr 13, 2013.

replies: 16people: 3as markdown or json

rh· Apr 7, 2013, 16:38 UTC · lore

segfault in git-remote-http

Hello,
(cross-posting to bitcoin-devel in case they've seen it)
 
1) git clone https://github.com/bitcoin/bitcoin.git
segfaulted in libcrypto.so.1.0.0
2) git clone git://github.com/bitcoin/bitcoin.git
had no problems
This was yesterday, I retried today
3) git clone https://github.com/bitcoin/bitcoin.git
and it worked fine.

Yesterday I was running bitcoin-0.8.1-linux/bitcoin-qt when #1 failed. Today I was running bitcoind when I ran #3

Two things I thought might be coming into play here, network pressure and memory pressure. Is this occurence reasonably possible or is something else afoot?

rh· Apr 7, 2013, 16:42 UTC · re: rh · lore
git version 1.7.12.1

On Sun, 7 Apr 2013 09:38:12 -0700 rh <richard_hubbe11@lavabit.com> wrote:

Show 20 quoted lines
> Hello,
> (cross-posting to bitcoin-devel in case they've seen it)
>  
> 1) git clone https://github.com/bitcoin/bitcoin.git
> segfaulted in libcrypto.so.1.0.0
> 
> 2) git clone git://github.com/bitcoin/bitcoin.git
> had no problems
> 
> This was yesterday, I retried today
> 
> 3) git clone https://github.com/bitcoin/bitcoin.git
> and it worked fine.
> 
> Yesterday I was running bitcoin-0.8.1-linux/bitcoin-qt when #1 failed.
> Today I was running bitcoind when I ran #3
> 
> Two things I thought might be coming into play here, network
> pressure and memory pressure. Is this occurence reasonably
> possible or is something else afoot?
rh· Apr 9, 2013, 15:47 UTC · re: rh · lore

On Sun, 7 Apr 2013 09:38:12 -0700 rh <richard_hubbe11@lavabit.com> wrote:

Show 21 quoted lines
> Hello,
> (cross-posting to bitcoin-devel in case they've seen it)
>  
> 1) git clone https://github.com/bitcoin/bitcoin.git
> segfaulted in libcrypto.so.1.0.0
> 
> 2) git clone git://github.com/bitcoin/bitcoin.git
> had no problems
> 
> This was yesterday, I retried today
> 
> 3) git clone https://github.com/bitcoin/bitcoin.git
> and it worked fine.
> 
> Yesterday I was running bitcoin-0.8.1-linux/bitcoin-qt when #1 failed.
> Today I was running bitcoind when I ran #3
> 
> Two things I thought might be coming into play here, network
> pressure and memory pressure. Is this occurence reasonably
> possible or is something else afoot?
> 
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.

Jeff King· Apr 9, 2013, 17:16 UTC · re: rh · lore
On Tue, Apr 09, 2013 at 08:47:18AM -0700, rh wrote:
Show 10 quoted lines
> 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
Daniel Stenberg· Apr 9, 2013, 17:25 UTC · re: Jeff King · lore
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
rh· Apr 12, 2013, 15:20 UTC · re: Daniel Stenberg · lore

On Tue, 9 Apr 2013 19:25:53 +0200 (CEST) Daniel Stenberg <daniel@haxx.se> wrote:

Show 10 quoted lines
> 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.
I posted to web.curl.general few days ago....
rh· Apr 9, 2013, 17:41 UTC · re: Jeff King · lore

On Tue, 9 Apr 2013 13:16:23 -0400 Jeff King <peff@peff.net> wrote:

Show 17 quoted lines
> 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.

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?

Jeff King· Apr 9, 2013, 17:47 UTC · re: rh · lore
On Tue, Apr 09, 2013 at 10:41:34AM -0700, rh wrote:
Show 7 quoted lines
> > 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
rh· Apr 9, 2013, 19:40 UTC · re: Jeff King · lore

On Tue, 9 Apr 2013 13:47:36 -0400 Jeff King <peff@peff.net> wrote:

Show 11 quoted lines
> 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
Yes they are hardlinks.
> 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 report....and 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 xxxxxxxx blah blah in libcrypto

Jeff King· Apr 10, 2013, 04:30 UTC · re: rh · lore
On Tue, Apr 09, 2013 at 12:40:44PM -0700, rh wrote:
Show 9 quoted lines
> > 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 report....and 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 xxxxxxxx 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
rh· Apr 10, 2013, 16:08 UTC · re: Jeff King · lore

On Wed, 10 Apr 2013 00:30:30 -0400 Jeff King <peff@peff.net> wrote:

Show 25 quoted lines
> 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 report....and 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 xxxxxxxx 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).

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.

Jeff King· Apr 10, 2013, 18:51 UTC · re: rh · lore
On Wed, Apr 10, 2013 at 09:08:50AM -0700, rh wrote:
Show 10 quoted lines
> > 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
Jeff King· Apr 10, 2013, 19:11 UTC · re: Jeff King · lore
On Wed, Apr 10, 2013 at 02:51:14PM -0400, Jeff King wrote:
Show 5 quoted lines
> 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/<pid>/comm  & /proc/<pid>/task/<tid>/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 0000000000400524 sp 00007fff46bb0700 error 6 in 12345678901234567890[400000+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
rh· Apr 10, 2013, 23:31 UTC · re: Jeff King · lore

On Wed, 10 Apr 2013 15:11:03 -0400 Jeff King <peff@peff.net> wrote:

Show 10 quoted lines
> 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
My guess was sort of close.
Show 27 quoted lines
> 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/<pid>/comm  & /proc/<pid>/task/<tid>/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
> 0000000000400524 sp 00007fff46bb0700 error 6 in 12345678901234567890
> [400000+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.

I cannot weigh in much on this other then to say it's interesting. I would think that changing git would be the right thing to do so that proper reports are made when users encounter errors that the kernel traps. Maybe making the important part first? https-remote-git? But I'm certain there's more to it than that!

But then again maybe the kernel should grab the entire command from /proc/<pid>/cmdline because it's really not providing distinct data.

rh· Apr 13, 2013, 01:01 UTC · re: Jeff King · lore

On Wed, 10 Apr 2013 15:11:03 -0400 Jeff King <peff@peff.net> wrote:

Show 37 quoted lines
> 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/<pid>/comm  & /proc/<pid>/task/<tid>/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
> 0000000000400524 sp 00007fff46bb0700 error 6 in 12345678901234567890
> [400000+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.

I realize this is not real critical but these three would all suffer the same fate on segfault. i.e. the kernel would report git-credential- as the app that suffered the seg fault.

git-credential-cache git-credential-cache--daemon git-credential-store

For sake of completeness on this thread if nothing else.
rh· Apr 10, 2013, 19:16 UTC · re: Jeff King · lore

On Wed, 10 Apr 2013 14:51:15 -0400 Jeff King <peff@peff.net> wrote:

Show 17 quoted lines
> 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.
Oh yeah, I didn't check $? but I got sidetracked seeing the segfault.
Show 6 quoted lines
> 
> 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.

This could be that it's using the first match or entry in the hardlink list. Which seems like a bad idea ... but that's a wild guess.

I agree it seems unlikely that it's related to the segfault.
Jeff King· Apr 10, 2013, 19:54 UTC · re: rh · lore
On Wed, Apr 10, 2013 at 12:16:25PM -0700, rh wrote:
Show 9 quoted lines
> > > 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

← back to recent threads