| From 2d9754744527708cf5a4636167f32433cabef34b Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 25 Feb 2020 10:44:27 -0800 |
| Subject: altera-stapl: altera_get_note: prevent write beyond end of 'key' |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Daniel Axtens <dja@axtens.net> |
| |
| [ Upstream commit 3745488e9d599916a0b40d45d3f30e3d4720288e ] |
| |
| altera_get_note is called from altera_init, where key is kzalloc(33). |
| |
| When the allocation functions are annotated to allow the compiler to see |
| the sizes of objects, and with FORTIFY_SOURCE, we see: |
| |
| In file included from drivers/misc/altera-stapl/altera.c:14:0: |
| In function ‘strlcpy’, |
| inlined from ‘altera_init’ at drivers/misc/altera-stapl/altera.c:2189:5: |
| include/linux/string.h:378:4: error: call to ‘__write_overflow’ declared with attribute error: detected write beyond size of object passed as 1st parameter |
| __write_overflow(); |
| ^~~~~~~~~~~~~~~~~~ |
| |
| That refers to this code in altera_get_note: |
| |
| if (key != NULL) |
| strlcpy(key, &p[note_strings + |
| get_unaligned_be32( |
| &p[note_table + (8 * i)])], |
| length); |
| |
| The error triggers because the length of 'key' is 33, but the copy |
| uses length supplied as the 'length' parameter, which is always |
| 256. Split the size parameter into key_len and val_len, and use the |
| appropriate length depending on what is being copied. |
| |
| Detected by compiler error, only compile-tested. |
| |
| Cc: "Igor M. Liplianin" <liplianin@netup.ru> |
| Signed-off-by: Daniel Axtens <dja@axtens.net> |
| Link: https://lore.kernel.org/r/20200120074344.504-2-dja@axtens.net |
| Signed-off-by: Kees Cook <keescook@chromium.org> |
| Link: https://lore.kernel.org/r/202002251042.D898E67AC@keescook |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/misc/altera-stapl/altera.c | 12 ++++++------ |
| 1 file changed, 6 insertions(+), 6 deletions(-) |
| |
| diff --git a/drivers/misc/altera-stapl/altera.c b/drivers/misc/altera-stapl/altera.c |
| index 494e263daa748..b7ee8043a133e 100644 |
| --- a/drivers/misc/altera-stapl/altera.c |
| +++ b/drivers/misc/altera-stapl/altera.c |
| @@ -2126,8 +2126,8 @@ static int altera_execute(struct altera_state *astate, |
| return status; |
| } |
| |
| -static int altera_get_note(u8 *p, s32 program_size, |
| - s32 *offset, char *key, char *value, int length) |
| +static int altera_get_note(u8 *p, s32 program_size, s32 *offset, |
| + char *key, char *value, int keylen, int vallen) |
| /* |
| * Gets key and value of NOTE fields in the JBC file. |
| * Can be called in two modes: if offset pointer is NULL, |
| @@ -2184,7 +2184,7 @@ static int altera_get_note(u8 *p, s32 program_size, |
| &p[note_table + (8 * i) + 4])]; |
| |
| if (value != NULL) |
| - strlcpy(value, value_ptr, length); |
| + strlcpy(value, value_ptr, vallen); |
| |
| } |
| } |
| @@ -2203,13 +2203,13 @@ static int altera_get_note(u8 *p, s32 program_size, |
| strlcpy(key, &p[note_strings + |
| get_unaligned_be32( |
| &p[note_table + (8 * i)])], |
| - length); |
| + keylen); |
| |
| if (value != NULL) |
| strlcpy(value, &p[note_strings + |
| get_unaligned_be32( |
| &p[note_table + (8 * i) + 4])], |
| - length); |
| + vallen); |
| |
| *offset = i + 1; |
| } |
| @@ -2463,7 +2463,7 @@ int altera_init(struct altera_config *config, const struct firmware *fw) |
| __func__, (format_version == 2) ? "Jam STAPL" : |
| "pre-standardized Jam 1.1"); |
| while (altera_get_note((u8 *)fw->data, fw->size, |
| - &offset, key, value, 256) == 0) |
| + &offset, key, value, 32, 256) == 0) |
| printk(KERN_INFO "%s: NOTE \"%s\" = \"%s\"\n", |
| __func__, key, value); |
| } |
| -- |
| 2.20.1 |
| |