Dave Jiang <dave.ji...@intel.com> wrote:
> + down_read(&key->sem);
> + payload = key->payload.data[0];
> + down_read(&user_key->sem);
> + upayload = user_key->payload.data[0];
Personally, I would do both downs first and then deref both payloads. The
compiler probably will be blocked from rearranging things to move the first
deref after the second down. Ideally it or the cpu should be able to move
things into the critical section, but the cpu barriers available may well
preclude that. That means that the compiler has to use a resource (stack/reg)
to stash the value across the second down.
> + * We don't need to release key->sem here because nvdimm_repalce_key
nvdimm_replace_key I presume.
> + sscanf(buf, "%s %u %u", cmd, &old_key, &new_key);
You should check that sscanf() returned 3.
Also, since there's no size limitation here on the cmd string, if someone, for
example, writes a string of SEC_CMD_SIZE lots of 'a', sscanf() will write all
of them into cmd[] and follow that with a NUL char, thereby overrunning your
cmd[] buffer. You need to either make the buffer one bigger or put a size
limit in sscanf string, e.g.:
if (sscanf(buf, "%" __stringify(SEC_CMD_SIZE) "s %u %u", ...) == 3)
-~-
Other than that, I think this is mostly right.
David
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
