Skip to content

Commit c72905e

Browse files
kv2019ilgirdwood
authored andcommitted
zephyr: syscall: sof_dma: fix block count validation in dma blk copy
deep_copy_dma_blk_cfg_list() does not check that number of entries in the linked list of DMA blocks matches cfg->block_count. This could be used to make kernel read from unvalidated user memory. Fix the issue by limiting list traversal to cfg->block_count. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
1 parent 88732a1 commit c72905e

1 file changed

Lines changed: 35 additions & 17 deletions

File tree

zephyr/syscall/sof_dma.c

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ static inline void z_vrfy_sof_dma_release_channel(struct sof_dma *dma,
111111
static inline struct dma_block_config *deep_copy_dma_blk_cfg_list(struct dma_config *cfg)
112112
{
113113
struct dma_block_config *kern_cfg;
114-
struct dma_block_config *kern_prev = NULL, *kern_next, *user_next;
114+
struct dma_block_config *kern_next, *user_next;
115115
size_t alloc_size;
116-
int i = 0;
116+
uint32_t i;
117117

118118
if (!cfg->block_count)
119119
return NULL;
@@ -131,17 +131,16 @@ static inline struct dma_block_config *deep_copy_dma_blk_cfg_list(struct dma_con
131131
if (!kern_cfg)
132132
return NULL;
133133

134-
for (user_next = cfg->head_block, kern_next = kern_cfg;
135-
user_next;
136-
user_next = user_next->next_block, kern_next++, i++) {
137-
if (i == cfg->block_count) {
138-
/* last block can point to first one */
139-
if (user_next != cfg->head_block)
140-
goto err;
141-
142-
kern_prev->next_block = kern_cfg;
143-
break;
144-
}
134+
user_next = cfg->head_block;
135+
for (i = 0, kern_next = kern_cfg; i < cfg->block_count; i++, kern_next++) {
136+
/*
137+
* The user list must contain exactly block_count entries.
138+
* A list that terminates early (NULL before the count is
139+
* reached) is rejected so the kernel copy and block_count
140+
* stay consistent and the driver never walks past the copy.
141+
*/
142+
if (!user_next)
143+
goto err;
145144

146145
if (k_usermode_from_copy(kern_next, user_next, sizeof(*kern_next)))
147146
goto err;
@@ -169,10 +168,29 @@ static inline struct dma_block_config *deep_copy_dma_blk_cfg_list(struct dma_con
169168
goto err;
170169
}
171170

172-
if (kern_prev)
173-
kern_prev->next_block = kern_next;
174-
175-
kern_prev = kern_next;
171+
/*
172+
* kern_next->next_block now holds the untrusted user-space
173+
* pointer copied above. Never let the kernel walk it: relink
174+
* every block to the kernel copy so the list can never point
175+
* outside the kern_cfg array.
176+
*/
177+
user_next = kern_next->next_block;
178+
179+
if (i + 1 < cfg->block_count) {
180+
/* link to the next kernel block */
181+
kern_next->next_block = kern_next + 1;
182+
} else {
183+
/*
184+
* Last block: the user list must end here, either
185+
* NULL-terminated or cyclic back to the first block.
186+
*/
187+
if (user_next == cfg->head_block)
188+
kern_next->next_block = kern_cfg;
189+
else if (!user_next)
190+
kern_next->next_block = NULL;
191+
else
192+
goto err;
193+
}
176194
}
177195

178196
/* set transfer list to point to first kernel transfer config object */

0 commit comments

Comments
 (0)