]>
Commit | Line | Data |
---|---|---|
d1cf1a50 GKH |
1 | From 3ab2011ea368ec3433ad49e1b9e1c7b70d2e65df Mon Sep 17 00:00:00 2001 |
2 | From: Tadeusz Struk <tadeusz.struk@intel.com> | |
3 | Date: Tue, 22 May 2018 14:37:18 -0700 | |
4 | Subject: tpm: fix race condition in tpm_common_write() | |
5 | ||
6 | From: Tadeusz Struk <tadeusz.struk@intel.com> | |
7 | ||
8 | commit 3ab2011ea368ec3433ad49e1b9e1c7b70d2e65df upstream. | |
9 | ||
10 | There is a race condition in tpm_common_write function allowing | |
11 | two threads on the same /dev/tpm<N>, or two different applications | |
12 | on the same /dev/tpmrm<N> to overwrite each other commands/responses. | |
13 | Fixed this by taking the priv->buffer_mutex early in the function. | |
14 | ||
15 | Also converted the priv->data_pending from atomic to a regular size_t | |
16 | type. There is no need for it to be atomic since it is only touched | |
17 | under the protection of the priv->buffer_mutex. | |
18 | ||
19 | Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") | |
20 | Cc: stable@vger.kernel.org | |
21 | Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com> | |
22 | Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> | |
23 | Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> | |
24 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
25 | ||
26 | --- | |
27 | drivers/char/tpm/tpm-dev-common.c | 40 +++++++++++++++++--------------------- | |
28 | drivers/char/tpm/tpm-dev.h | 2 - | |
29 | 2 files changed, 19 insertions(+), 23 deletions(-) | |
30 | ||
31 | --- a/drivers/char/tpm/tpm-dev-common.c | |
32 | +++ b/drivers/char/tpm/tpm-dev-common.c | |
33 | @@ -37,7 +37,7 @@ static void timeout_work(struct work_str | |
34 | struct file_priv *priv = container_of(work, struct file_priv, work); | |
35 | ||
36 | mutex_lock(&priv->buffer_mutex); | |
37 | - atomic_set(&priv->data_pending, 0); | |
38 | + priv->data_pending = 0; | |
39 | memset(priv->data_buffer, 0, sizeof(priv->data_buffer)); | |
40 | mutex_unlock(&priv->buffer_mutex); | |
41 | } | |
42 | @@ -46,7 +46,6 @@ void tpm_common_open(struct file *file, | |
43 | struct file_priv *priv) | |
44 | { | |
45 | priv->chip = chip; | |
46 | - atomic_set(&priv->data_pending, 0); | |
47 | mutex_init(&priv->buffer_mutex); | |
48 | setup_timer(&priv->user_read_timer, user_reader_timeout, | |
49 | (unsigned long)priv); | |
50 | @@ -59,29 +58,24 @@ ssize_t tpm_common_read(struct file *fil | |
51 | size_t size, loff_t *off) | |
52 | { | |
53 | struct file_priv *priv = file->private_data; | |
54 | - ssize_t ret_size; | |
55 | - ssize_t orig_ret_size; | |
56 | + ssize_t ret_size = 0; | |
57 | int rc; | |
58 | ||
59 | del_singleshot_timer_sync(&priv->user_read_timer); | |
60 | flush_work(&priv->work); | |
61 | - ret_size = atomic_read(&priv->data_pending); | |
62 | - if (ret_size > 0) { /* relay data */ | |
63 | - orig_ret_size = ret_size; | |
64 | - if (size < ret_size) | |
65 | - ret_size = size; | |
66 | + mutex_lock(&priv->buffer_mutex); | |
67 | ||
68 | - mutex_lock(&priv->buffer_mutex); | |
69 | + if (priv->data_pending) { | |
70 | + ret_size = min_t(ssize_t, size, priv->data_pending); | |
71 | rc = copy_to_user(buf, priv->data_buffer, ret_size); | |
72 | - memset(priv->data_buffer, 0, orig_ret_size); | |
73 | + memset(priv->data_buffer, 0, priv->data_pending); | |
74 | if (rc) | |
75 | ret_size = -EFAULT; | |
76 | ||
77 | - mutex_unlock(&priv->buffer_mutex); | |
78 | + priv->data_pending = 0; | |
79 | } | |
80 | ||
81 | - atomic_set(&priv->data_pending, 0); | |
82 | - | |
83 | + mutex_unlock(&priv->buffer_mutex); | |
84 | return ret_size; | |
85 | } | |
86 | ||
87 | @@ -92,17 +86,19 @@ ssize_t tpm_common_write(struct file *fi | |
88 | size_t in_size = size; | |
89 | ssize_t out_size; | |
90 | ||
91 | + if (in_size > TPM_BUFSIZE) | |
92 | + return -E2BIG; | |
93 | + | |
94 | + mutex_lock(&priv->buffer_mutex); | |
95 | + | |
96 | /* Cannot perform a write until the read has cleared either via | |
97 | * tpm_read or a user_read_timer timeout. This also prevents split | |
98 | * buffered writes from blocking here. | |
99 | */ | |
100 | - if (atomic_read(&priv->data_pending) != 0) | |
101 | + if (priv->data_pending != 0) { | |
102 | + mutex_unlock(&priv->buffer_mutex); | |
103 | return -EBUSY; | |
104 | - | |
105 | - if (in_size > TPM_BUFSIZE) | |
106 | - return -E2BIG; | |
107 | - | |
108 | - mutex_lock(&priv->buffer_mutex); | |
109 | + } | |
110 | ||
111 | if (copy_from_user | |
112 | (priv->data_buffer, (void __user *) buf, in_size)) { | |
113 | @@ -133,7 +129,7 @@ ssize_t tpm_common_write(struct file *fi | |
114 | return out_size; | |
115 | } | |
116 | ||
117 | - atomic_set(&priv->data_pending, out_size); | |
118 | + priv->data_pending = out_size; | |
119 | mutex_unlock(&priv->buffer_mutex); | |
120 | ||
121 | /* Set a timeout by which the reader must come claim the result */ | |
122 | @@ -150,5 +146,5 @@ void tpm_common_release(struct file *fil | |
123 | del_singleshot_timer_sync(&priv->user_read_timer); | |
124 | flush_work(&priv->work); | |
125 | file->private_data = NULL; | |
126 | - atomic_set(&priv->data_pending, 0); | |
127 | + priv->data_pending = 0; | |
128 | } | |
129 | --- a/drivers/char/tpm/tpm-dev.h | |
130 | +++ b/drivers/char/tpm/tpm-dev.h | |
131 | @@ -8,7 +8,7 @@ struct file_priv { | |
132 | struct tpm_chip *chip; | |
133 | ||
134 | /* Data passed to and from the tpm via the read/write calls */ | |
135 | - atomic_t data_pending; | |
136 | + size_t data_pending; | |
137 | struct mutex buffer_mutex; | |
138 | ||
139 | struct timer_list user_read_timer; /* user needs to claim result */ |