On Thu, 2016-07-21 at 09:30 +0200, Rabin Vincent wrote:
> On Wed, Jul 20, 2016 at 02:57:29PM +0100, Sachin Prabhu wrote:
> >
> > Rabin, do you have a reliable reproducer for this case? If yes, can
> > you
> > please point me to it.
> I have only seen one report of the oops in real testing, so I don't
> have
> a case that would reproduce easily on an unmodified kernel, but while
> investigating this I made a debug patch which forces the faulty
> sequence
> to occur and provides a 100% reliable reproducer.
>
> The below patched forces the race condition by making two mount.cifs
> processes wait for each other in the appropriate places.
>
> With the force-race patch applied and without this fix, the crash
> occurs
> every time when the following sequence of commands is run. Tested in
> a
> KVM guest on latest mainline with my "unbreak TCP session reuse"
> patch
> applied.
>
> With the force-race patch applied and with this fix, the mount
> processes
> hang since the race condition can never be satisfied.
Thanks Rabin.
Sachin Prabhu
>
> cp /sbin/mount.cifs /sbin/cifs1
> cp /sbin/mount.cifs /sbin/cifs2
> mkdir -p cifs cifs2
> cifs1 //192.168.1.1/test cifs -o credentials=/creds &
> sleep 2
> cifs2 //192.168.1.1/test2 cifs2 -o credentials=/creds
>
> 8<-------------------------
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 8347c90..1ed6dfd 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -35,6 +35,32 @@
> #include <linux/highmem.h>
> #include <crypto/skcipher.h>
>
> +static struct completion completions[100];
> +
> +void hack_set_state(int newstate)
> +{
> + printk("%s: caller %pF sets state to %d\n", current->comm,
> (void *)_RET_IP_, newstate);
> + complete(&completions[newstate]);
> +}
> +
> +void hack_wait_for_state(int state)
> +{
> + printk("%s caller %pF wants state %d\n", current->comm,
> (void *)_RET_IP_, state);
> + wait_for_completion(&completions[state]);
> + printk("%s caller %pF got state %d\n", current->comm, (void
> *)_RET_IP_, state);
> +}
> +
> +static int hack_init(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(completions); i++)
> + init_completion(&completions[i]);
> +
> + return 0;
> +}
> +late_initcall(hack_init);
> +
> static int
> cifs_crypto_shash_md5_allocate(struct TCP_Server_Info *server)
> {
> @@ -54,6 +80,7 @@ cifs_crypto_shash_md5_allocate(struct
> TCP_Server_Info *server)
>
> size = sizeof(struct shash_desc) +
> crypto_shash_descsize(server->secmech.md5);
> +
> server->secmech.sdescmd5 = kmalloc(size, GFP_KERNEL);
> if (!server->secmech.sdescmd5) {
> crypto_free_shash(server->secmech.md5);
> @@ -661,8 +688,10 @@ static int crypto_hmacmd5_alloc(struct
> TCP_Server_Info *server)
> unsigned int size;
>
> /* check if already allocated */
> - if (server->secmech.sdeschmacmd5)
> + if (server->secmech.sdeschmacmd5) {
> + printk("%s: server %p already allocated\n",
> __func__, server);
> return 0;
> + }
>
> server->secmech.hmacmd5 = crypto_alloc_shash("hmac(md5)", 0,
> 0);
> if (IS_ERR(server->secmech.hmacmd5)) {
> @@ -674,12 +703,28 @@ static int crypto_hmacmd5_alloc(struct
> TCP_Server_Info *server)
>
> size = sizeof(struct shash_desc) +
> crypto_shash_descsize(server-
> >secmech.hmacmd5);
> +
> + if (!strcmp(current->comm, "cifs1")) {
> + hack_wait_for_state(10);
> + }
> + if (!strcmp(current->comm, "cifs2")) {
> + hack_set_state(10);
> + hack_wait_for_state(20);
> + }
> +
> server->secmech.sdeschmacmd5 = kmalloc(size, GFP_KERNEL);
> + printk("%s: %s: server %p alloc sdeschmacmd5=%p\n",
> __func__, current->comm, server, server->secmech.sdeschmacmd5);
> if (!server->secmech.sdeschmacmd5) {
> crypto_free_shash(server->secmech.hmacmd5);
> server->secmech.hmacmd5 = NULL;
> return -ENOMEM;
> }
> +
> + if (!strcmp(current->comm, "cifs2")) {
> + hack_set_state(30);
> + hack_wait_for_state(40);
> + }
> +
> server->secmech.sdeschmacmd5->shash.tfm = server-
> >secmech.hmacmd5;
> server->secmech.sdeschmacmd5->shash.flags = 0x0;
>
> @@ -745,6 +790,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const
> struct nls_table *nls_cp)
>
> mutex_lock(&ses->server->srv_mutex);
>
> + printk("%s: ses %p ses->server %p\n", __func__, ses, ses-
> >server);
> rc = crypto_hmacmd5_alloc(ses->server);
> if (rc) {
> cifs_dbg(VFS, "could not crypto alloc hmacmd5 rc
> %d\n", rc);
> @@ -780,6 +826,11 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const
> struct nls_table *nls_cp)
> goto unlock;
> }
>
> + if (!strcmp(current->comm, "cifs1")) {
> + hack_set_state(20);
> + hack_wait_for_state(30);
> + }
> +
> rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5-
> >shash,
> ntlmv2->ntlmv2_hash,
> CIFS_HMAC_MD5_HASH_SIZE);
> @@ -788,6 +839,8 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const
> struct nls_table *nls_cp)
> goto unlock;
> }
>
> + hack_set_state(40);
> +
> rc = crypto_shash_final(&ses->server->secmech.sdeschmacmd5-
> >shash,
> ses->auth_key.response);
> if (rc)
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
