Skip to content

Fix Server Crash in LCS Command#7375

Merged
antirez merged 2 commits intoredis:unstablefrom
hwware:lcs_crash_fix
Jun 12, 2020
Merged

Fix Server Crash in LCS Command#7375
antirez merged 2 commits intoredis:unstablefrom
hwware:lcs_crash_fix

Conversation

@hwware
Copy link
Contributor

@hwware hwware commented Jun 9, 2020

This PR fixes a server crash error for STRALGO LCS command, it happens when user specifies non string type object associated with KEYS option. Therefore we need to check whether the provided value of KEYS option is string type. reproduce:

127.0.0.1:6379> hset foo1 bar 1 ttt rrr
(integer) 0
127.0.0.1:6379> hset bar1 bar 1 ttt rrr
(integer) 0
127.0.0.1:6379> STRALGO LCS keys foo1 bar1
Could not connect to Redis at 127.0.0.1:6379: Connection refused
not connected>

Crash Log:

=== REDIS BUG REPORT START: Cut & paste starting from here ===
14070:M 08 Jun 2020 23:59:48.834 # ------------------------------------------------
14070:M 08 Jun 2020 23:59:48.834 # !!! Software Failure. Press left mouse button to continue
14070:M 08 Jun 2020 23:59:48.834 # Guru Meditation: Unknown encoding type #object.c:534
14070:M 08 Jun 2020 23:59:48.834 # (forcing SIGSEGV in order to print the stack trace)
14070:M 08 Jun 2020 23:59:48.834 # ------------------------------------------------
14070:M 08 Jun 2020 23:59:48.834 # Redis 999.999.999 crashed by signal: 11
14070:M 08 Jun 2020 23:59:48.834 # Crashed running the instruction at: 0x10f350a9e
14070:M 08 Jun 2020 23:59:48.834 # Accessing address: 0xffffffffffffffff
14070:M 08 Jun 2020 23:59:48.834 # Failed assertion: (:0)

------ STACK TRACE ------
EIP:
0 redis-server 0x000000010f350a9e _serverPanic + 334

Backtrace:
0 redis-server 0x000000010f35354e logStackTrace + 110
1 redis-server 0x000000010f35390c sigsegvHandler + 236
2 libsystem_platform.dylib 0x00007fff8e30cb3a _sigtramp + 26
3 ??? 0x0000000000000400 0x0 + 1024
4 redis-server 0x000000010f31e18a getDecodedObject + 202
5 redis-server 0x000000010f33523e stralgoLCS + 222
6 redis-server 0x000000010f305ffb call + 347
7 redis-server 0x000000010f306b78 processCommand + 1912
8 redis-server 0x000000010f319727 processInputBuffer + 407
9 redis-server 0x000000010f3aab00 connSocketEventHandler + 304
10 redis-server 0x000000010f2fd877 aeProcessEvents + 807
11 redis-server 0x000000010f2fdbbd aeMain + 29
12 redis-server 0x000000010f30a09e main + 1918
13 libdyld.dylib 0x00007fff8e0fd235 start + 1
14 ??? 0x0000000000000001 0x0 + 1

------ INFO OUTPUT ------

Server

redis_version:999.999.999
redis_git_sha1:48b2915c
redis_git_dirty:0
redis_build_id:3ac4d049bacc7d05
redis_mode:standalone
os:Darwin 16.7.0 x86_64
arch_bits:64
multiplexing_api:kqueue
atomicvar_api:atomic-builtin
gcc_version:4.2.1
process_id:14070
run_id:d52ca7d9fddac41d9650e98ba90eb027cadd1a83
tcp_port:6379
uptime_in_seconds:42
uptime_in_days:0
hz:10
configured_hz:10
lru_clock:14616884
executable:/Users/danieldai/Documents/hwware/redis/src/./redis-server
config_file:

Clients

connected_clients:1
client_recent_max_input_buffer:2
client_recent_max_output_buffer:0
blocked_clients:0
tracking_clients:0
clients_in_timeout_table:0

Memory

used_memory:1063560
used_memory_human:1.01M
used_memory_rss:4255744
used_memory_rss_human:4.06M
used_memory_peak:1063560
used_memory_peak_human:1.01M
used_memory_peak_perc:100.16%
used_memory_overhead:1016786
used_memory_startup:999464
used_memory_dataset:46774
used_memory_dataset_perc:72.97%
allocator_allocated:1216128
allocator_active:1454080
allocator_resident:4308992
total_system_memory:17179869184
total_system_memory_human:16.00G
used_memory_lua:37888
used_memory_lua_human:37.00K
used_memory_scripts:0
used_memory_scripts_human:0B
number_of_cached_scripts:0
maxmemory:0
maxmemory_human:0B
maxmemory_policy:noeviction
allocator_frag_ratio:1.20
allocator_frag_bytes:237952
allocator_rss_ratio:2.96
allocator_rss_bytes:2854912
rss_overhead_ratio:0.99
rss_overhead_bytes:-53248
mem_fragmentation_ratio:4.17
mem_fragmentation_bytes:3234792
mem_not_counted_for_evict:0
mem_replication_backlog:0
mem_clients_slaves:0
mem_clients_normal:16986
mem_aof_buffer:0
mem_allocator:jemalloc-5.1.0
active_defrag_running:0
lazyfree_pending_objects:0

Persistence

loading:0
rdb_changes_since_last_save:2
rdb_bgsave_in_progress:0
rdb_last_save_time:1591675146
rdb_last_bgsave_status:ok
rdb_last_bgsave_time_sec:-1
rdb_current_bgsave_time_sec:-1
rdb_last_cow_size:0
aof_enabled:0
aof_rewrite_in_progress:0
aof_rewrite_scheduled:0
aof_last_rewrite_time_sec:-1
aof_current_rewrite_time_sec:-1
aof_last_bgrewrite_status:ok
aof_last_write_status:ok
aof_last_cow_size:0
module_fork_in_progress:0
module_fork_last_cow_size:0

Stats

total_connections_received:1
total_commands_processed:3
instantaneous_ops_per_sec:0
total_net_input_bytes:189
total_net_output_bytes:18543
instantaneous_input_kbps:0.00
instantaneous_output_kbps:0.00
rejected_connections:0
sync_full:0
sync_partial_ok:0
sync_partial_err:0
expired_keys:0
expired_stale_perc:0.00
expired_time_cap_reached_count:0
expire_cycle_cpu_milliseconds:0
evicted_keys:0
keyspace_hits:2
keyspace_misses:0
pubsub_channels:0
pubsub_patterns:0
latest_fork_usec:0
migrate_cached_sockets:0
slave_expires_tracked_keys:0
active_defrag_hits:0
active_defrag_misses:0
active_defrag_key_hits:0
active_defrag_key_misses:0
tracking_total_keys:0
tracking_total_items:0
tracking_total_prefixes:0
unexpected_error_replies:0

Replication

role:master
connected_slaves:0
master_replid:0e77fe777bae3dfa99c66541d625aae5789cc7d5
master_replid2:0000000000000000000000000000000000000000
master_repl_offset:0
second_repl_offset:-1
repl_backlog_active:0
repl_backlog_size:1048576
repl_backlog_first_byte_offset:0
repl_backlog_histlen:0

CPU

used_cpu_sys:0.026122
used_cpu_user:0.027866
used_cpu_sys_children:0.000000
used_cpu_user_children:0.000000

Modules

Commandstats

cmdstat_hset:calls=2,usec=56,usec_per_call=28.00
cmdstat_command:calls=1,usec=1422,usec_per_call=1422.00

Cluster

cluster_enabled:0

Keyspace

db0:keys=6,expires=0,avg_ttl=0

------ CLIENT LIST OUTPUT ------
id=4 addr=127.0.0.1:49906 fd=8 name= age=39 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=56 qbuf-free=32712 obl=0 oll=0 omem=0 events=r cmd=stralgo user=default

------ CURRENT CLIENT INFO ------
id=4 addr=127.0.0.1:49906 fd=8 name= age=39 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=56 qbuf-free=32712 obl=0 oll=0 omem=0 events=r cmd=stralgo user=default
argv[0]: 'STRALGO'
argv[1]: 'LCS'
argv[2]: 'keys'
argv[3]: 'foo1'
argv[4]: 'bar1'

------ REGISTERS ------
14070:M 08 Jun 2020 23:59:48.835 #
RAX:20bad09b37900028 RBX:000000010f456e94
RCX:0000220000002303 RDX:0000000000012068
RDI:00007fff97000f78 RSI:000000000011bd00
RBP:00007fff50906640 RSP:00007fff50906450
R8 :0000000000000040 R9 :00007fff97000f70
R10:ffffffffffffffff R11:0000000000012068
R12:0000000000000004 R13:000000010febfcc0
R14:0000000000000216 R15:000000010f44dbfb
RIP:000000010f350a9e EFL:0000000000010206
CS :000000000000002b FS:0000000000000000 GS:0000000000000000
14070:M 08 Jun 2020 23:59:48.835 # (00007fff5090645f) -> 000000010f4028bf
14070:M 08 Jun 2020 23:59:48.835 # (00007fff5090645e) -> 000000010f81a490
14070:M 08 Jun 2020 23:59:48.835 # (00007fff5090645d) -> 000000010f811270
14070:M 08 Jun 2020 23:59:48.835 # (00007fff5090645c) -> 000000010f807b40
14070:M 08 Jun 2020 23:59:48.835 # (00007fff5090645b) -> 000000010f40340e
14070:M 08 Jun 2020 23:59:48.835 # (00007fff5090645a) -> 00007fff50906510
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906459) -> 000000010f81a4c8
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906458) -> 000000010f81a4b8
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906457) -> 000000010f81a4b0
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906456) -> 000000010f81a4a8
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906455) -> 000000010f301950
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906454) -> 0000000000000031
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906453) -> 000000000df09345
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906452) -> 000000010f81a470
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906451) -> 000000010f81a4a8
14070:M 08 Jun 2020 23:59:48.835 # (00007fff50906450) -> 000000010f81a4b8

------ MODULES INFO OUTPUT ------

------ DUMPING CODE AROUND EIP ------
Symbol: _serverPanic (base: 0x10f350950)
Module: /Users/danieldai/Documents/hwware/redis/src/./redis-server (base 0x10f2f9000)
$ xxd -r -p /tmp/dump.hex /tmp/dump.bin
$ objdump --adjust-vma=0x10f350950 -D -b binary -m i386:x86-64 /tmp/dump.bin

14070:M 08 Jun 2020 23:59:48.835 # dump of function (hexdump of 462 bytes):
554889e541574156534881ecd80100004889d34189f64989ff84c074380f298540feffff0f298d50feffff0f299560feffff0f299d70feffff0f29a580feffff0f29ad90feffff0f29b5a0feffff0f29bdb0feffff4c898d38feffff4c898530feffff48898d28feffff488b0547961100488b00488945e0488d8510feffff488945d0488d4510488945c8c745c430000000c745c018000000488dbdc0feffff4c8d4dc0be0001000031d2b9000100004989d8e868320f00488d1d99f0130083bb940d000000751b488d352f651000bf03040000e8f708fbffc783940d000001000000488d1d5a641000bf0300000031c04889dee8e70afbff488d3575641000bf0300000031c0e8d40afbff488d359c641000488d95c0feffffbf0300000031c04c89f94589f0e8b40afbff488d3597641000bf0300000031c0e8a10afbffbf0300000031c04889dee8920afbffc60425ffffffff78488b055b951100488b00483b45e0750e4881c4d80100005b415e415f5dc3e883310f000f1f8000000000554889e54157415641554154534881ecd8000000488b051d951100488b00488945d0c747100000000048c74708000000004889bd20ffffff48c70700000000488d1592ef13008b82c007000085c0
Function at 0x10f443c70 is je_witness_postfork_child
Function at 0x10f301320 is serverLogRaw
Function at 0x10f301530 is serverLog
Function at 0x10f443c4c is je_witness_postfork_child

=== REDIS BUG REPORT END. Make sure to include from START to END. ===

   Please report the crash by opening an issue on github:

       http://github.com/antirez/redis/issues

Suspect RAM error? Use redis-server --test-memory to verify it.

Segmentation fault: 11

@soloestoy
Copy link
Contributor

Good catch, but the memory leak fix is not complete, to fix it I think we can replace return statement in for loop and ambiguous parameters check with goto cleanup;, and cleanup can be defined at the end of stralgoLCS.

Ping @antirez

@hwware
Copy link
Contributor Author

hwware commented Jun 11, 2020

hi @soloestoy, thank you for your comment, you are right, I pushed the changes based on your suggestions...please review... thanks :)

@antirez
Copy link
Contributor

antirez commented Jun 12, 2020

Thanks @hwware @soloestoy, looks good to me, merging.

@antirez antirez merged commit a66298e into redis:unstable Jun 12, 2020
@antirez
Copy link
Contributor

antirez commented Jun 12, 2020

@hwware the only problem was that lookup() can return NULL.

@antirez
Copy link
Contributor

antirez commented Jun 12, 2020

See 1055398

@hwware
Copy link
Contributor Author

hwware commented Jun 12, 2020

thanks @antirez ,I saw your commit and it make sense indeed. firstly we need to add the check for NULL object in the if in order to make sure NULL type handles correctly. also only the call getDecodedObject increase the reference count so we don't need to decrease the ref count if we did not call it.

JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants