Re: [PATCH] fs/coda: ensure the header peeked at is the same in the actual message

From: Jan Harkes
Date: Tue Sep 26 2017 - 15:35:33 EST


On Sat, Sep 23, 2017 at 10:35:45PM -0400, Meng Xu wrote:
> Hi Jaharkes and Coda filesystem developers,
>
> I am resending the email on a potential race condition bug I found in the
> Coda filesystem as well as the patch I propose. Please feel free to comment
> whether you think this is a serious problem and whether the patch will work.
> Thank you.

Hi,

It does look like there is a potential race there but I don't believe it
is very serious because,

- The userspace Coda cache manager component (Coda client) is using
co-routines instead of true multithreading. So even if someone tries
to exploit this through a vulnerability through the Coda client the
whole process is blocked on the write systemcall and there is no other
thread of execution available that can try to change the fields.

- If someone can exploit the Coda client, they already have achieved
root, so no need to try to exploit anything in the kernel.

- If someone can replace the opcode and unique id, they can replace
anything else in the message, with much worse results. For instance by
rewriting the response to an open call from success to failure the
attacker is able to cause a file descriptor leak because the Coda
client believes there is an open file, while the kernel and
application believe the open failed, etc.

Now there are two types of messages the Coda client writes to the kernel
device.

- Downcalls which provide hints to trigger cache revalidations, here the
unique id is ignored and the opcode mostly indicates how severe the
cache invalidation is (only a single object, whole subtree, etc.) If
someone wants to mess with that rewriting the file identifier that is
in the message body is more useful and the worst they can do is make
stale data stick in the kernel cache. I notice that your patch does
not address the copy_from_user for downcalls around line 128.

- The other type are the upcall responses, file system requests are
blocked until the Coda client returns a response with the matching
unique id from the request. This match is performed at line 158, which
is after the peek, but before the second copy_from_user. Once the
matching request has been found and dequeued we do not look at the
unique id anymore. Because the response is correlated to a specific
request and we do not re-copy the opcode from the response to the
req->uc_opcode field it doesn't really matter what the opcode in the
response was at this point because all decisions are based on the
request opcode.

Jan