This repository has been archived by the owner on Feb 20, 2021. It is now read-only.
3 2 #548
Open
3 2 #548
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
My guess was that wait3() with WNOHANG could never return -1 and an error. However issue redis#2897 may possibly indicate that this could happen under non clear conditions. While we try to understand this better, better to handle a return value of -1 explicitly, otherwise in the case a BGREWRITE is in progress but wait3() returns -1, the effect is to match the first branch of the if/else block since server.rdb_child_pid is -1, and call backgroundSaveDoneHandler() without a good reason, that will, in turn, crash the Redis server with an assertion.
There are some cases of printing unsigned integer with %d conversion specificator and vice versa (signed integer with %u specificator). Patch by Sergey Polovko. Backported to Redis from Disque.
Now we have a single function to call in any state of the slave handshake, instead of using different functions for different states which is error prone. Change performed in the context of issue redis#2479 but does not fix it, since should be functionally identical to the past. Just an attempt to make replication.c simpler to follow.
Some time ago I broken replicas migration (reported in redis#2924). The idea was to prevent masters without replicas from getting replicas because of replica migration, I remember it to create issues with tests, but there is no clue in the commit message about why it was so undesirable. However my patch as a side effect totally ruined the concept of replicas migration since we want it to work also for instances that, technically, never had slaves in the past: promoted slaves. So now instead the ability to be targeted by replicas migration, is a new flag "migrate-to". It only applies to masters, and is set in the following two cases: 1. When a master gets a slave, it is set. 2. When a slave turns into a master because of fail over, it is set. This way replicas migration targets are only masters that used to have slaves, and slaves of masters (that used to have slaves... obviously) and are promoted. The new flag is only internal, and is never exposed in the output nor persisted in the nodes configuration, since all the information to handle it are implicit in the cluster configuration we already have.
The old version was modeled with two failovers, however after the first it is possible that another slave will migrate to the new master, since for some time the new master is not backed by any slave. Probably there should be some pause after a failover, before the migration. Anyway the test is simpler in this way, and depends less on timing.
We wait a fixed amount of time (5 seconds currently) much greater than the usual Cluster node to node communication latency, before migrating. This way when a failover occurs, before detecting the new master as a target for migration, we give the time to its natural slaves (the slaves of the failed over master) to announce they switched to the new master, preventing an useless migration operation.
…ds. Defaults to 15000 milliseconds
We need to process replies after errors in order to delete keys successfully transferred. Also argument rewriting was fixed since it was broken in several ways. Now a fresh argument vector is created and set if we are acknowledged of at least one key.
We use the new variadic/pipelined MIGRATE for faster migration. Testing is not easy because to see the time it takes for a slot to be migrated requires a very large data set, but even with all the overhead of migrating multiple slots and to setup them properly, what used to take 4 seconds (1 million keys, 200 slots migrated) is now 1.6 which is a good improvement. However the improvement can be a lot larger if: 1. We use large datasets where a single slot has many keys. 2. By moving more than 10 keys per iteration, making this configurable, which is planned. Close redis#2710 Close redis#2711
The old test, designed to do a transformation on the bits that was invertible, in order to avoid touching the original memory content, was not effective as it was redis-server --test-memory. The former often reported OK while the latter was able to spot the error. So the test was substituted with one that may perform better, however the new one must backup the memory tested, so it tests memory in small pieces. This limits the effectiveness because of the CPU caches. However some attempt is made in order to trash the CPU cache between the fill and the check stages, but not for the addressing test unfortunately. We'll see if this test will be able to find errors where the old failed.
This feature is useful, especially in deployments using Sentinel in order to setup Redis HA, where the slave is executed with NAT or port forwarding, so that the auto-detected port/ip addresses, as listed in the "INFO replication" output of the master, or as provided by the "ROLE" command, don't match the real addresses at which the slave is reachable for connections.
The problem was fixed in antirez/linenoise repository applying a patch contributed by @lamby. Here the new version is updated in the Redis source tree. Close redis#1418 Close redis#3322
Fix warning: ISO C does not support '__FUNCTION__' predefined identifier [-Wpedantic]
…-trib.rb Display the nodes summary once the cluster is established using redis-trib.rb After the cluster meet and join was done, when the summary was shown, it was giving info regarding the nodes. This fix ensures that confusion where the slaves were shown as masters. Fix would be to reset the nodes and reload the cluster information before checking the cluster status after creating it.
This commit attempts to fix a problem with PR redis#3467.
This commit fixes a vunlerability reported by Cory Duplantis
of Cisco Talos, see TALOS-2016-0206 for reference.
CONFIG SET client-output-buffer-limit accepts as client class "master"
which is actually only used to implement CLIENT KILL. The "master" class
has ID 3. What happens is that the global structure:
server.client_obuf_limits[class]
Is accessed with class = 3. However it is a 3 elements array, so writing
the 4th element means to write up to 24 bytes of memory *after* the end
of the array, since the structure is defined as:
typedef struct clientBufferLimitsConfig {
unsigned long long hard_limit_bytes;
unsigned long long soft_limit_bytes;
time_t soft_limit_seconds;
} clientBufferLimitsConfig;
EVALUATION OF IMPACT:
Checking what's past the boundaries of the array in the global
'server' structure, we find AOF state fields:
clientBufferLimitsConfig client_obuf_limits[CLIENT_TYPE_OBUF_COUNT];
/* AOF persistence */
int aof_state; /* AOF_(ON|OFF|WAIT_REWRITE) */
int aof_fsync; /* Kind of fsync() policy */
char *aof_filename; /* Name of the AOF file */
int aof_no_fsync_on_rewrite; /* Don't fsync if a rewrite is in prog. */
int aof_rewrite_perc; /* Rewrite AOF if % growth is > M and... */
off_t aof_rewrite_min_size; /* the AOF file is at least N bytes. */
off_t aof_rewrite_base_size; /* AOF size on latest startup or rewrite. */
off_t aof_current_size; /* AOF current size. */
Writing to most of these fields should be harmless and only cause problems in
Redis persistence that should not escalate to security problems.
However unfortunately writing to "aof_filename" could be potentially a
security issue depending on the access pattern.
Searching for "aof.filename" accesses in the source code returns many different
usages of the field, including using it as input for open(), logging to the
Redis log file or syslog, and calling the rename() syscall.
It looks possible that attacks could lead at least to informations
disclosure of the state and data inside Redis. However note that the
attacker must already have access to the server. But, worse than that,
it looks possible that being able to change the AOF filename can be used
to mount more powerful attacks: like overwriting random files with AOF
data (easily a potential security issue as demostrated here:
http://antirez.com/news/96), or even more subtle attacks where the
AOF filename is changed to a path were a malicious AOF file is loaded
in order to exploit other potential issues when the AOF parser is fed
with untrusted input (no known issue known currently).
The fix checks the places where the 'master' class is specifiedf in
order to access configuration data structures, and return an error in
this cases.
WHO IS AT RISK?
The "master" client class was introduced in Redis in Jul 28 2015.
Every Redis instance released past this date is not vulnerable
while all the releases after this date are. Notably:
Redis 3.0.x is NOT vunlerable.
Redis 3.2.x IS vulnerable.
Redis unstable is vulnerable.
In order for the instance to be at risk, at least one of the following
conditions must be true:
1. The attacker can access Redis remotely and is able to send
the CONFIG SET command (often banned in managed Redis instances).
2. The attacker is able to control the "redis.conf" file and
can wait or trigger a server restart.
The problem was fixed 26th September 2016 in all the releases affected.
Compiling Redis worked as a side effect of jemalloc target specifying -ldl as needed linker options, otherwise it is not provided during linking and dlopen() API will remain unresolved symbols.
A bug was reported in the context in issue redis#3631. The root cause of the bug was that certain neighbor boxes were zeroed after the "inside the bounding box or not" check, simply because the bounding box computation function was wrong. A few debugging infos where enhanced and moved in other parts of the code. A check to avoid steps=0 was added, but is unrelated to this issue and I did not verified it was an actual bug in practice.
The test now uses more diverse radius sizes, especially sizes near or greater the whole earth surface are used, that are known to trigger edge cases. Moreover the PRNG seeding was probably resulting into the same sequence tested over and over again, now seeding unsing the current unix time in milliseconds. Related to redis#3631.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.