Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

plugin-pay: SEGV in direct_pay_listpeerchannels when field private missing #7197

Closed
ksedgwic opened this issue Apr 3, 2024 · 4 comments · Fixed by #7235
Closed

plugin-pay: SEGV in direct_pay_listpeerchannels when field private missing #7197

ksedgwic opened this issue Apr 3, 2024 · 4 comments · Fixed by #7235
Labels
crash
Milestone
v24.05

Comments

@ksedgwic
Copy link
Collaborator

ksedgwic commented Apr 3, 2024 •
edited
Loading

Issue and Steps to Reproduce

Running v24.02.1-18-gab4ea82 (v24.02.1 + VLS mods)

IMPORTANT - this has @rustyrussell 's pay routing fixes, this is something new ...

Node has almost 80 channels, CLBOSS is operating so maybe is balancing or swaps ...

plugin-pay died:

Tue 2024-04-02 19:47:51 PDT home4 lightningd[918393]: 2024-04-03T02:47:51.496Z INFO    plugin-pay: Killing plugin: exited during normal operation
Tue 2024-04-02 19:47:51 PDT home4 lightningd[918393]: 2024-04-03T02:47:51.496Z **BROKEN** plugin-pay: Plugin marked as important, shutting down lightningd!

corefile is present, backtrace:

Reading symbols from /usr/local/libexec/c-lightning/plugins/pay...
[New LWP 918406]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/local/libexec/c-lightning/plugins/pay'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x0000558b197e5781 in send_backtrace (why=0x7ffcd006b3c0 "FATAL SIGNAL 11") at common/daemon.c:36
#2  0x0000558b197e5945 in crashdump (sig=11) at common/daemon.c:75
#3  <signal handler called>
#4  0x0000558b197ed346 in json_to_bool (
    buffer=0x7fecb6d03038 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#37784\",\"result\":{\"channels\":[{\"peer_id\":\"02720bde32af57f55c94ead55d8e2119deed5b9dcde1d76aded588e3e114743797\",\"peer_connected\":true,\"reestablished\":true,\"c"..., tok=0x0, b=0x558b1e4dbc4a) at common/json_parse_simple.c:160
#5  0x0000558b197c4ce1 in json_to_listpeers_channel (ctx=0xx558b1e4e5b48, 
    buffer=0x7fecb6d03038 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#37784\",\"result\":{\"channels\":[{\"peer_id\":\"02720bde32af57f55c94ead55d8e2119deed5b9dcde1d76aded588e3e114743797\",\"peer_connected\":true,\"reestablished\":true,\"c"..., tok=0x7feca601eac0) at plugins/libplugin.c:2118
#6  0x0000558b197c5024 in json_to_listpeers_channels (ctx=0x558b1ad77498, 
    buffer=0x7fecb6d03038 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#37784\",\"result\":{\"channels\":[{\"peer_id\":\"02720bde32af57f55c94ead55d8e2119deed5b9dcde1d76aded588e3e114743797\",\"peer_connected\":true,\"reestablished\":true,\"c"..., tok=0x7feca60100b0) at plugins/libplugin.c:2179
#7  0x0000558b197cf4f9 in direct_pay_listpeerchannels (cmd=0x0, 
    buffer=0x7fecb6d03038 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#37784\",\"result\":{\"channels\":[{\"peer_id\":\"02720bde32af57f55c94ead55d8e2119deed5b9dcde1d76aded588e3e114743797\",\"peer_connected\":true,\"reestablished\":true,\"c"..., toks=0x7feca60100b0, p=0x558b1e5c7b38)
    at plugins/libplugin-pay.c:3360
#8  0x0000558b197c0757 in handle_rpc_reply (plugin=0x558b1ad774c8, toks=0x7feca6010038) at plugins/libplugin.c:961
#9  0x0000558b197c1462 in rpc_read_response_one (plugin=0x558b1ad774c8) at plugins/libplugin.c:1148
#10 0x0000558b197c15e5 in rpc_conn_read_response (conn=0x558b1ad78b18, plugin=0x558b1ad774c8) at plugins/libplugin.c:1172
#11 0x0000558b1993d70c in next_plan (conn=0x558b1ad78b18, plan=0x558b1ad78b38) at ccan/ccan/io/io.c:59
#12 0x0000558b1993e341 in do_plan (conn=0x558b1ad78b18, plan=0x558b1ad78b38, idle_on_epipe=false) at ccan/ccan/io/io.c:407
#13 0x0000558b1993e383 in io_ready (conn=0x558b1ad78b18, pollflags=1) at ccan/ccan/io/io.c:417
#14 0x0000558b1994071f in io_loop (timers=0x558b1ad77608, expired=0x7ffcd006bec0) at ccan/ccan/io/poll.c:453
#15 0x0000558b197c4a75 in plugin_main (argv=0x7ffcd006c168, init=0x558b197bb7bb <init>, restartability=PLUGIN_RESTARTABLE, init_rpc=true, features=0x0, commands=0x558b19aaa3e0 <commands>, num_commands=3, notif_subs=0x0, num_notif_subs=0, hook_subs=0x0, num_hook_subs=0, notif_topics=0x558b19aac020 <notification_topics>, num_notif_topics=2) at plugins/libplugin.c:2086
#16 0x0000558b197bdfc5 in main (argc=1, argv=0x7ffcd006c168) at plugins/pay.c:1303
(gdb)

Looks like the problem happened here:

	json_to_bool(buffer, privtok, &chan->private);

Feels like maybe the buffer is not correct? Here is the buffer text data extracted via gdb:


buffer.txt

Corefile and logs all still available, let me know if additional stuff needed ...
@ksedgwic ksedgwic mentioned this issue Apr 6, 2024
Closed
@ksedgwic
Copy link
Collaborator Author

ksedgwic commented Apr 6, 2024

I think this is relevant:
37ccca5

@ksedgwic ksedgwic changed the title plugin-pay: SEGV below direct_pay_listpeerchannels plugin-pay: SEGV in direct_pay_listpeerchannels when field private missing Apr 6, 2024
@vincenzopalazzo vincenzopalazzo added this to the v24.05 milestone Apr 7, 2024
@ksedgwic ksedgwic mentioned this issue Apr 8, 2024
Merged
@king-11
Copy link
Contributor

king-11 commented Apr 15, 2024

I also faced this backtrace as below:

CLN Version: v24.02.2

#0  0x0000000000000000 in ?? ()
#1  0x0000559b0a4a3330 in send_backtrace (why=why@entry=0x7ffec20f3470 "FATAL SIGNAL 11") at common/daemon.c:36
#2  0x0000559b0a4a3398 in crashdump (sig=11) at common/daemon.c:75
#3  <signal handler called>
#4  json_to_bool (
    buffer=buffer@entry=0x7f10b1dff2a8 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#249951\",\"result\":{\"channels\":[{\"peer_id\":\"031adb9eb2d66693f85fa31a4adca0319ba68219f3ad5f9a2ef9b34a6b40755fa1\",\"peer_connected\":true,\"reestablished\":true,\""..., tok=tok@entry=0x0, b=b@entry=0x7f10aaffb65a) at common/json_parse_simple.c:160
#5  0x0000559b0a489929 in json_to_listpeers_channel (ctx=0x7f10ab8375e8,
    buffer=buffer@entry=0x7f10b1dff2a8 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#249951\",\"result\":{\"channels\":[{\"peer_id\":\"031adb9eb2d66693f85fa31a4adca0319ba68219f3ad5f9a2ef9b34a6b40755fa1\",\"peer_connected\":true,\"reestablished\":true,\""..., tok=tok@entry=0x7f10ab1993e8) at plugins/libplugin.c:2118
#6  0x0000559b0a48d289 in json_to_listpeers_channels (ctx=<optimized out>,
    buffer=buffer@entry=0x7f10b1dff2a8 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#249951\",\"result\":{\"channels\":[{\"peer_id\":\"031adb9eb2d66693f85fa31a4adca0319ba68219f3ad5f9a2ef9b34a6b40755fa1\",\"peer_connected\":true,\"reestablished\":true,\""..., tok=tok@entry=0x7f10ab1943c0) at plugins/libplugin.c:2179
#7  0x0000559b0a4948d6 in direct_pay_listpeerchannels (cmd=0x0,
    buffer=0x7f10b1dff2a8 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"pay:listpeerchannels#249951\",\"result\":{\"channels\":[{\"peer_id\":\"031adb9eb2d66693f85fa31a4adca0319ba68219f3ad5f9a2ef9b34a6b40755fa1\",\"peer_connected\":true,\"reestablished\":true,\""..., toks=0x7f10ab1943c0, p=0x7f10ab3ac718) at plugins/libplugin-pay.c:3360
#8  0x0000559b0a48cbe9 in handle_rpc_reply (plugin=plugin@entry=0x7f10bd3b90e8, toks=0x7f10ab194348)
    at plugins/libplugin.c:961
#9  0x0000559b0a48cd8c in rpc_read_response_one (plugin=plugin@entry=0x7f10bd3b90e8) at plugins/libplugin.c:1148
#10 0x0000559b0a48ce39 in rpc_conn_read_response (conn=<optimized out>, plugin=0x7f10bd3b90e8)
    at plugins/libplugin.c:1172
#11 0x0000559b0a5e0390 in next_plan (conn=conn@entry=0x7f10bd2fa548, plan=plan@entry=0x7f10bd2fa568)
    at ccan/ccan/io/io.c:59
#12 0x0000559b0a5e0817 in do_plan (conn=conn@entry=0x7f10bd2fa548, plan=plan@entry=0x7f10bd2fa568,
    idle_on_epipe=idle_on_epipe@entry=false) at ccan/ccan/io/io.c:407
#13 0x0000559b0a5e08b0 in io_ready (conn=conn@entry=0x7f10bd2fa548, pollflags=5) at ccan/ccan/io/io.c:417
#14 0x0000559b0a5e210d in io_loop (timers=timers@entry=0x7f10bd3b9228, expired=expired@entry=0x7ffec20f4378)
    at ccan/ccan/io/poll.c:453
#15 0x0000559b0a48d1b8 in plugin_main (argv=argv@entry=0x7ffec20f4568, init=init@entry=0x559b0a488664 <init>,
    restartability=restartability@entry=PLUGIN_RESTARTABLE, init_rpc=init_rpc@entry=true,
    features=features@entry=0x0, commands=commands@entry=0x559b0a744f80 <commands>, num_commands=3, notif_subs=0x0,
    num_notif_subs=0, hook_subs=0x0, num_hook_subs=0, notif_topics=0x559b0a747010 <notification_topics>,
    num_notif_topics=2) at plugins/libplugin.c:2086
#16 0x0000559b0a4887ec in main (argc=<optimized out>, argv=0x7ffec20f4568) at plugins/pay.c:1303

@endothermicdev
Copy link
Collaborator

I think an unsaved dual open channel could have produced a listpeerchannels output that would cause this. We can add the private bool to the unsaved channel's listpeerchannels output, but that might be misleading. I'm leaning toward just ignoring any uncommitted channels here as there's no sense in trying to route through them.

endothermicdev added a commit to endothermicdev/lightning that referenced this issue Apr 16, 2024
@endothermicdev
pay: ignore uncommited channels in listpeerchannels output
2271f2e 
Fixes ElementsProject#7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
endothermicdev added a commit to endothermicdev/lightning that referenced this issue Apr 16, 2024
@endothermicdev
pay: ignore uncommited channels in listpeerchannels output
bf8737a 
Fixes ElementsProject#7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
@endothermicdev
Copy link
Collaborator

I went through the buffer you provided - it was actually a regular uncommitted channel. Still I think it makes more sense to skip these entirely since they will be missing many fields an active channel would have.
{
"peer_id": "038cd9f3679d5b39bb2105978467918d549572de472f07dd729e37c7a6377d41d5",
"peer_connected": true,
"state": "OPENINGD",
"owner": "lightning_openingd",
"opener": "local",
"to_us_msat": 8317559000,
"total_msat": 8317559000,
"features": [
"option_static_remotekey",
"option_anchors_zero_fee_htlc_tx"
]
}

This probably deserves a blackbox test too.

endothermicdev added a commit to endothermicdev/lightning that referenced this issue Apr 18, 2024
@endothermicdev
pay: ignore uncommited channels in listpeerchannels output
efdf01d 
Uncommited channels are missing several fields which would normally be
populated by an active channel.  This simply skips them for the purposes
of finding a route.  The particular culprit was:
{
  "peer_id": "038cd9f3679d5b39bb2105978467918d549572de472f07dd729e37c7a6377d41d5",
  "peer_connected": true,
  "state": "OPENINGD",
  "owner": "lightning_openingd",
  "opener": "local",
  "to_us_msat": 8317559000,
  "total_msat": 8317559000,
  "features": [
    "option_static_remotekey",
    "option_anchors_zero_fee_htlc_tx"
  ]
}

Fixes ElementsProject#7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
@endothermicdev endothermicdev mentioned this issue Apr 18, 2024
Merged
endothermicdev added a commit to endothermicdev/lightning that referenced this issue Apr 20, 2024
@endothermicdev
pay: ignore uncommited channels in listpeerchannels output
75d3c29 
Uncommited channels are missing several fields which would normally be
populated by an active channel.  This simply skips them for the purposes
of finding a route.  The particular culprit was:
{
  "peer_id": "038cd9f3679d5b39bb2105978467918d549572de472f07dd729e37c7a6377d41d5",
  "peer_connected": true,
  "state": "OPENINGD",
  "owner": "lightning_openingd",
  "opener": "local",
  "to_us_msat": 8317559000,
  "total_msat": 8317559000,
  "features": [
    "option_static_remotekey",
    "option_anchors_zero_fee_htlc_tx"
  ]
}

Fixes ElementsProject#7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
endothermicdev added a commit to endothermicdev/lightning that referenced this issue Apr 20, 2024
@endothermicdev
pay: ignore uncommited channels in listpeerchannels output
2dae71d 
Uncommited channels are missing several fields which would normally be
populated by an active channel.  This simply skips them for the purposes
of finding a route.  The particular culprit was:
{
  "peer_id": "038cd9f3679d5b39bb2105978467918d549572de472f07dd729e37c7a6377d41d5",
  "peer_connected": true,
  "state": "OPENINGD",
  "owner": "lightning_openingd",
  "opener": "local",
  "to_us_msat": 8317559000,
  "total_msat": 8317559000,
  "features": [
    "option_static_remotekey",
    "option_anchors_zero_fee_htlc_tx"
  ]
}

Fixes ElementsProject#7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
endothermicdev added a commit that referenced this issue Apr 30, 2024
@endothermicdev
pay: ignore uncommited channels in listpeerchannels output
ff7efec 
Uncommited channels are missing several fields which would normally be
populated by an active channel.  This simply skips them for the purposes
of finding a route.  The particular culprit was:
{
  "peer_id": "038cd9f3679d5b39bb2105978467918d549572de472f07dd729e37c7a6377d41d5",
  "peer_connected": true,
  "state": "OPENINGD",
  "owner": "lightning_openingd",
  "opener": "local",
  "to_us_msat": 8317559000,
  "total_msat": 8317559000,
  "features": [
    "option_static_remotekey",
    "option_anchors_zero_fee_htlc_tx"
  ]
}

Fixes #7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
rustyrussell pushed a commit to rustyrussell/lightning that referenced this issue May 14, 2024
@endothermicdev @rustyrussell
pay: ignore uncommited channels in listpeerchannels output
732dd62 
Uncommited channels are missing several fields which would normally be
populated by an active channel.  This simply skips them for the purposes
of finding a route.  The particular culprit was:
{
  "peer_id": "038cd9f3679d5b39bb2105978467918d549572de472f07dd729e37c7a6377d41d5",
  "peer_connected": true,
  "state": "OPENINGD",
  "owner": "lightning_openingd",
  "opener": "local",
  "to_us_msat": 8317559000,
  "total_msat": 8317559000,
  "features": [
    "option_static_remotekey",
    "option_anchors_zero_fee_htlc_tx"
  ]
}

Fixes ElementsProject#7197 - SEGV in direct_pay_listpeerchannels when field private missing

Changelog-Fixed: Fixed crash in pay plugin caused by parsing uncommitted dual open channels
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
crash
Projects
None yet
Milestone
v24.05
Development

Successfully merging a pull request may close this issue.

Pay listpeerchannels fix endothermicdev/lightning
4 participants
@ksedgwic @vincenzopalazzo @king-11 @endothermicdev