[MODULES] Calling RedisModule_WrongArity from a thread-safe context crashes redis

[dvirsky]

To reproduce:

1. Block the client.
2. Create a thread safe context from the blocked client.
3. Call RedisModule_WrongArity(ctx)

The reason is that the function does the following:

 addReplyErrorFormat(ctx->client,
        "wrong number of arguments for '%s' command",
        (char*)ctx->client->argv[0]->ptr);

and we do not have argv at this point.
I think the simplest thing to do would be to keep a pointer to the command name (with incrRef) directly in the context and not rely on argv and the client. It might come in handy in other places as well.

[dvirsky]

cc: @mnunberg if you have anything to add to this...

[mnunberg]

This is because it tries to dereference the `argv[0]` in the internal client in order to get the original command name. Because the internal client in the thread safe context lacks the array, it crashes. It seems we can either Omit the command name from the error message itself Copy just the command name (wasteful IMO) Make the argv array itself thread safe (read-only though) - this would also allow us to avoid internal copying especially during indexing.

…

On Mar 15, 2018, at 6:08 AM, Dvir Volk ***@***.***> wrote: cc: @mnunberg <https://github.com/mnunberg> if you have anything to add to this... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4756 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAanUb8fKN2FKKxde0gyCmYg5YAR3DNwks5tej2xgaJpZM4Sr3Pe>.

[dvirsky]

Keep in mind that we don't have to actually copy the string. We can copy the robj and increase the ref count, which is very cheap.

[jonahharris]

Agreed. @dvirsky can you share your exact example case. I want to run some tests on different methods regarding the fix.

[dvirsky]

@jonahharris I don't have anything ATM, we had it in redisearch but fixed it. You can modify the module examples in the redis source tree to reproduce this pretty easily. try helloblock.c I think.