Skip to content

Commit

Permalink
Do security attack check only when command not found to reduce the cr…
Browse files Browse the repository at this point in the history
…itical path (valkey-io#1212)

When explored the cycles distribution for main thread with io-threads
enabled. We found this security attack check takes significant time in
main thread, **~3%** cycles were used to do the commands security check
in main thread.

This patch try to completely avoid doing it in the hot path. We can do
it only after we looked up the command and it wasn't found, just before
we call commandCheckExistence.

---------

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>
Co-authored-by: Wangyang Guo <wangyang.guo@intel.com>
  • Loading branch information
lipzhu and guowangy authored Oct 25, 2024
1 parent 55bbbe0 commit 9c60fcd
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -3916,12 +3916,6 @@ int processCommand(client *c) {
reqresAppendRequest(c);
}

/* Handle possible security attacks. */
if (!strcasecmp(c->argv[0]->ptr, "host:") || !strcasecmp(c->argv[0]->ptr, "post")) {
securityWarningCommand(c);
return C_ERR;
}

/* If we're inside a module blocked context yielding that wants to avoid
* processing clients, postpone the command. */
if (server.busy_module_yield_flags != BUSY_MODULE_YIELD_NONE &&
Expand All @@ -3936,6 +3930,13 @@ int processCommand(client *c) {
* we do not have to repeat the same checks */
if (!client_reprocessing_command) {
struct serverCommand *cmd = c->io_parsed_cmd ? c->io_parsed_cmd : lookupCommand(c->argv, c->argc);
if (!cmd) {
/* Handle possible security attacks. */
if (!strcasecmp(c->argv[0]->ptr, "host:") || !strcasecmp(c->argv[0]->ptr, "post")) {
securityWarningCommand(c);
return C_ERR;
}
}
c->cmd = c->lastcmd = c->realcmd = cmd;
sds err;
if (!commandCheckExistence(c, &err)) {
Expand Down

0 comments on commit 9c60fcd

Please sign in to comment.