| Message ID | 1644849974-8043-2-git-send-email-quic_deesin@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V4,1/2] soc: qcom: smem: map only partitions used by local HOST | expand |
Hi Deepak,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.17-rc4 next-20220215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Deepak-Kumar-Singh/soc-qcom-smem-map-only-partitions-used-by-local-HOST/20220214-224750
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 754e0b0e35608ed5206d6a67a791563c631cec07
config: openrisc-randconfig-s031-20220214 (https://download.01.org/0day-ci/archive/20220215/202202152150.EZ8yJDzm-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/cfc33be784b2bfdafba0ae278dfbf92bdd9111da
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Deepak-Kumar-Singh/soc-qcom-smem-map-only-partitions-used-by-local-HOST/20220214-224750
git checkout cfc33be784b2bfdafba0ae278dfbf92bdd9111da
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/soc/qcom/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/soc/qcom/smem.c:430:16: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_header *header @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:430:16: sparse: expected struct smem_header *header
drivers/soc/qcom/smem.c:430:16: sparse: got void [noderef] __iomem *virt_base
drivers/soc/qcom/smem.c:517:16: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_header *header @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:517:16: sparse: expected struct smem_header *header
drivers/soc/qcom/smem.c:517:16: sparse: got void [noderef] __iomem *virt_base
drivers/soc/qcom/smem.c:534:50: sparse: sparse: incorrect type in return expression (different address spaces) @@ expected void * @@ got void [noderef] __iomem * @@
drivers/soc/qcom/smem.c:534:50: sparse: expected void *
drivers/soc/qcom/smem.c:534:50: sparse: got void [noderef] __iomem *
>> drivers/soc/qcom/smem.c:695:22: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_partition_header *phdr @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:695:22: sparse: expected struct smem_partition_header *phdr
drivers/soc/qcom/smem.c:695:22: sparse: got void [noderef] __iomem *virt_base
>> drivers/soc/qcom/smem.c:699:27: sparse: sparse: cast to restricted __le32
>> drivers/soc/qcom/smem.c:699:27: sparse: sparse: cast to restricted __le32
>> drivers/soc/qcom/smem.c:699:27: sparse: sparse: cast to restricted __le32
>> drivers/soc/qcom/smem.c:699:27: sparse: sparse: cast to restricted __le32
>> drivers/soc/qcom/smem.c:699:27: sparse: sparse: cast to restricted __le32
>> drivers/soc/qcom/smem.c:699:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:703:22: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_partition_header *phdr @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:703:22: sparse: expected struct smem_partition_header *phdr
drivers/soc/qcom/smem.c:703:22: sparse: got void [noderef] __iomem *virt_base
drivers/soc/qcom/smem.c:707:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:707:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:707:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:707:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:707:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:707:27: sparse: sparse: cast to restricted __le32
drivers/soc/qcom/smem.c:710:24: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_header *header @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:710:24: sparse: expected struct smem_header *header
drivers/soc/qcom/smem.c:710:24: sparse: got void [noderef] __iomem *virt_base
drivers/soc/qcom/smem.c:723:30: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/soc/qcom/smem.c:723:30: sparse: void *
drivers/soc/qcom/smem.c:723:30: sparse: void [noderef] __iomem *
drivers/soc/qcom/smem.c:744:36: sparse: sparse: subtraction of different types can't work (different address spaces)
drivers/soc/qcom/smem.c:753:28: sparse: sparse: subtraction of different types can't work (different address spaces)
drivers/soc/qcom/smem.c:762:36: sparse: sparse: subtraction of different types can't work (different address spaces)
drivers/soc/qcom/smem.c:777:16: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_header *header @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:777:16: sparse: expected struct smem_header *header
drivers/soc/qcom/smem.c:777:16: sparse: got void [noderef] __iomem *virt_base
drivers/soc/qcom/smem.c:810:57: sparse: sparse: restricted __le32 degrades to integer
drivers/soc/qcom/smem.c:831:16: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_partition_header *header @@ got void [noderef] __iomem * @@
drivers/soc/qcom/smem.c:831:16: sparse: expected struct smem_partition_header *header
drivers/soc/qcom/smem.c:831:16: sparse: got void [noderef] __iomem *
drivers/soc/qcom/smem.c:982:22: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_ptable *ptable @@ got void [noderef] __iomem * @@
drivers/soc/qcom/smem.c:982:22: sparse: expected struct smem_ptable *ptable
drivers/soc/qcom/smem.c:982:22: sparse: got void [noderef] __iomem *
drivers/soc/qcom/smem.c:1091:16: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct smem_header *header @@ got void [noderef] __iomem *virt_base @@
drivers/soc/qcom/smem.c:1091:16: sparse: expected struct smem_header *header
drivers/soc/qcom/smem.c:1091:16: sparse: got void [noderef] __iomem *virt_base
>> drivers/soc/qcom/smem.c:1112:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got restricted __le32 * @@
drivers/soc/qcom/smem.c:1112:31: sparse: expected void const volatile [noderef] __iomem *addr
drivers/soc/qcom/smem.c:1112:31: sparse: got restricted __le32 *
drivers/soc/qcom/smem.c:1112:67: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got restricted __le32 * @@
drivers/soc/qcom/smem.c:1112:67: sparse: expected void const volatile [noderef] __iomem *addr
drivers/soc/qcom/smem.c:1112:67: sparse: got restricted __le32 *
drivers/soc/qcom/smem.c: note: in included file (through arch/openrisc/include/asm/io.h, include/linux/io.h):
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
include/asm-generic/io.h:267:16: sparse: sparse: cast to restricted __le32
vim +695 drivers/soc/qcom/smem.c
4b638df4c9d556 Bjorn Andersson 2015-06-26 675
4b638df4c9d556 Bjorn Andersson 2015-06-26 676 /**
4b638df4c9d556 Bjorn Andersson 2015-06-26 677 * qcom_smem_get_free_space() - retrieve amount of free space in a partition
4b638df4c9d556 Bjorn Andersson 2015-06-26 678 * @host: the remote processor identifying a partition, or -1
4b638df4c9d556 Bjorn Andersson 2015-06-26 679 *
4b638df4c9d556 Bjorn Andersson 2015-06-26 680 * To be used by smem clients as a quick way to determine if any new
4b638df4c9d556 Bjorn Andersson 2015-06-26 681 * allocations has been made.
4b638df4c9d556 Bjorn Andersson 2015-06-26 682 */
4b638df4c9d556 Bjorn Andersson 2015-06-26 683 int qcom_smem_get_free_space(unsigned host)
4b638df4c9d556 Bjorn Andersson 2015-06-26 684 {
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 685 struct smem_partition *part;
4b638df4c9d556 Bjorn Andersson 2015-06-26 686 struct smem_partition_header *phdr;
4b638df4c9d556 Bjorn Andersson 2015-06-26 687 struct smem_header *header;
4b638df4c9d556 Bjorn Andersson 2015-06-26 688 unsigned ret;
4b638df4c9d556 Bjorn Andersson 2015-06-26 689
4b638df4c9d556 Bjorn Andersson 2015-06-26 690 if (!__smem)
4b638df4c9d556 Bjorn Andersson 2015-06-26 691 return -EPROBE_DEFER;
4b638df4c9d556 Bjorn Andersson 2015-06-26 692
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 693 if (host < SMEM_HOST_COUNT && __smem->partitions[host].virt_base) {
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 694 part = &__smem->partitions[host];
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 @695 phdr = part->virt_base;
9806884d8cd552 Stephen Boyd 2015-09-02 696 ret = le32_to_cpu(phdr->offset_free_cached) -
9806884d8cd552 Stephen Boyd 2015-09-02 697 le32_to_cpu(phdr->offset_free_uncached);
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 698
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 @699 if (ret > le32_to_cpu(part->size))
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 700 return -EINVAL;
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 701 } else if (__smem->global_partition.virt_base) {
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 702 part = &__smem->global_partition;
70716a4ee6c89c Deepak Kumar Singh 2022-02-14 703 phdr = part->virt_base;
d52e404874369f Chris Lew 2017-10-11 704 ret = le32_to_cpu(phdr->offset_free_cached) -
d52e404874369f Chris Lew 2017-10-11 705 le32_to_cpu(phdr->offset_free_uncached);
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 706
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 707 if (ret > le32_to_cpu(part->size))
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 708 return -EINVAL;
4b638df4c9d556 Bjorn Andersson 2015-06-26 709 } else {
4b638df4c9d556 Bjorn Andersson 2015-06-26 710 header = __smem->regions[0].virt_base;
9806884d8cd552 Stephen Boyd 2015-09-02 711 ret = le32_to_cpu(header->available);
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 712
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 713 if (ret > __smem->regions[0].size)
cfc33be784b2bf Deepak Kumar Singh 2022-02-14 714 return -EINVAL;
4b638df4c9d556 Bjorn Andersson 2015-06-26 715 }
4b638df4c9d556 Bjorn Andersson 2015-06-26 716
4b638df4c9d556 Bjorn Andersson 2015-06-26 717 return ret;
4b638df4c9d556 Bjorn Andersson 2015-06-26 718 }
4b638df4c9d556 Bjorn Andersson 2015-06-26 719 EXPORT_SYMBOL(qcom_smem_get_free_space);
4b638df4c9d556 Bjorn Andersson 2015-06-26 720
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Mon 14 Feb 06:46 PST 2022, Deepak Kumar Singh wrote: > Structures in shared memory that can be modified by remote > processors may have untrusted values, they should be validated > before use. > > Adding proper validation before using fields of shared > structures. I'm not able to find patch 1/2, did you send it out or is it just me being unlucky finding it? > > Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com> > --- > drivers/soc/qcom/smem.c | 81 +++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 68 insertions(+), 13 deletions(-) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index 96444ff..644844b 100644 > --- a/drivers/soc/qcom/smem.c > +++ b/drivers/soc/qcom/smem.c > @@ -367,13 +367,18 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem, > struct smem_partition_header *phdr; > size_t alloc_size; > void *cached; > + void *p_end; > > phdr = (struct smem_partition_header __force *)part->virt_base; > + p_end = (void *)phdr + part->size; > > hdr = phdr_to_first_uncached_entry(phdr); > end = phdr_to_last_uncached_entry(phdr); > cached = phdr_to_last_cached_entry(phdr); > > + if (WARN_ON((void *)end > p_end || (void *)cached > p_end)) cached is a void * already, do you really need to cast it? > + return -EINVAL; > + > while (hdr < end) { > if (hdr->canary != SMEM_PRIVATE_CANARY) > goto bad_canary; > @@ -383,6 +388,9 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem, > hdr = uncached_entry_next(hdr); > } > > + if (WARN_ON((void *)hdr > p_end)) > + return -EINVAL; > + > /* Check that we don't grow into the cached region */ > alloc_size = sizeof(*hdr) + ALIGN(size, 8); > if ((void *)hdr + alloc_size > cached) { > @@ -501,6 +509,8 @@ static void *qcom_smem_get_global(struct qcom_smem *smem, > struct smem_header *header; > struct smem_region *region; > struct smem_global_entry *entry; > + u64 entry_offset; > + u32 e_size; > u32 aux_base; > unsigned i; > > @@ -515,9 +525,13 @@ static void *qcom_smem_get_global(struct qcom_smem *smem, > region = &smem->regions[i]; > > if ((u32)region->aux_base == aux_base || !aux_base) { > + e_size = le32_to_cpu(entry->size); > + entry_offset = le32_to_cpu(entry->offset); > + > if (size != NULL) > - *size = le32_to_cpu(entry->size); > - return region->virt_base + le32_to_cpu(entry->offset); > + *size = e_size; > + > + return region->virt_base + entry_offset; The only change I see here is that you read entry->size regardless of size being requested or not, so I don't see any "sanity checking" here. > } > } > > @@ -531,8 +545,12 @@ static void *qcom_smem_get_private(struct qcom_smem *smem, > { > struct smem_private_entry *e, *end; > struct smem_partition_header *phdr; > + void *item_ptr, *p_end; > + u32 padding_data; > + u32 e_size; > > phdr = (struct smem_partition_header __force *)part->virt_base; > + p_end = (void *)phdr + part->size; > > e = phdr_to_first_uncached_entry(phdr); > end = phdr_to_last_uncached_entry(phdr); > @@ -542,36 +560,65 @@ static void *qcom_smem_get_private(struct qcom_smem *smem, > goto invalid_canary; > > if (le16_to_cpu(e->item) == item) { > - if (size != NULL) > - *size = le32_to_cpu(e->size) - > - le16_to_cpu(e->padding_data); > + if (size != NULL) { > + e_size = le32_to_cpu(e->size); > + padding_data = le16_to_cpu(e->padding_data); > > - return uncached_entry_to_item(e); > + if (WARN_ON(e_size > part->size || padding_data > e_size)) > + return ERR_PTR(-EINVAL); > + > + *size = e_size - padding_data; > + } > + > + item_ptr = uncached_entry_to_item(e); > + if (WARN_ON(item_ptr > p_end)) > + return ERR_PTR(-EINVAL); > + > + return item_ptr; > } > > e = uncached_entry_next(e); > } > > + if (WARN_ON((void *)e > p_end)) > + return ERR_PTR(-EINVAL); > + > /* Item was not found in the uncached list, search the cached list */ > > e = phdr_to_first_cached_entry(phdr, part->cacheline); > end = phdr_to_last_cached_entry(phdr); > > + if (WARN_ON((void *)e < (void *)phdr || (void *)end > p_end)) > + return ERR_PTR(-EINVAL); > + > while (e > end) { > if (e->canary != SMEM_PRIVATE_CANARY) > goto invalid_canary; > > if (le16_to_cpu(e->item) == item) { > - if (size != NULL) > - *size = le32_to_cpu(e->size) - > - le16_to_cpu(e->padding_data); > + if (size != NULL) { > + e_size = le32_to_cpu(e->size); > + padding_data = le16_to_cpu(e->padding_data); > + > + if (WARN_ON(e_size > part->size || padding_data > e_size)) > + return ERR_PTR(-EINVAL); > + > + *size = e_size - padding_data; > + } > + > + item_ptr = cached_entry_to_item(e); > + if (WARN_ON(item_ptr < (void *)phdr)) > + return ERR_PTR(-EINVAL); > > - return cached_entry_to_item(e); > + return item_ptr; > } > > e = cached_entry_next(e, part->cacheline); > } > > + if (WARN_ON((void *)e < (void *)phdr)) > + return ERR_PTR(-EINVAL); > + > return ERR_PTR(-ENOENT); > > invalid_canary: > @@ -648,14 +695,23 @@ int qcom_smem_get_free_space(unsigned host) > phdr = part->virt_base; > ret = le32_to_cpu(phdr->offset_free_cached) - > le32_to_cpu(phdr->offset_free_uncached); > + > + if (ret > le32_to_cpu(part->size)) > + return -EINVAL; > } else if (__smem->global_partition.virt_base) { > part = &__smem->global_partition; > phdr = part->virt_base; > ret = le32_to_cpu(phdr->offset_free_cached) - > le32_to_cpu(phdr->offset_free_uncached); > + > + if (ret > le32_to_cpu(part->size)) > + return -EINVAL; > } else { > header = __smem->regions[0].virt_base; > ret = le32_to_cpu(header->available); > + > + if (ret > __smem->regions[0].size) > + return -EINVAL; > } > > return ret; > @@ -918,13 +974,12 @@ qcom_smem_enumerate_partitions(struct qcom_smem *smem, u16 local_host) > static int qcom_smem_map_toc(struct qcom_smem *smem, struct smem_region *region) I presume this function was introduced in patch 1? > { > u32 ptable_start; > - int ret; Below changes doesn't affect "ret", so it should probably have been removed in the previous patch. > > /* map starting 4K for smem header */ > - region->virt_base = devm_ioremap_wc(dev, region->aux_base, SZ_4K); > + region->virt_base = devm_ioremap_wc(smem->dev, region->aux_base, SZ_4K); I don't see "dev" in the scope here, did this compile after patch 1? > ptable_start = region->aux_base + region->size - SZ_4K; > /* map last 4k for toc */ > - smem->ptable = devm_ioremap_wc(dev, ptable_start, SZ_4K); > + smem->ptable = devm_ioremap_wc(smem->dev, ptable_start, SZ_4K); Ditto. Regards, Bjorn > > if (!region->virt_base || !smem->ptable) > return -ENOMEM; > -- > 2.7.4 >
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c index 96444ff..644844b 100644 --- a/drivers/soc/qcom/smem.c +++ b/drivers/soc/qcom/smem.c @@ -367,13 +367,18 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem, struct smem_partition_header *phdr; size_t alloc_size; void *cached; + void *p_end; phdr = (struct smem_partition_header __force *)part->virt_base; + p_end = (void *)phdr + part->size; hdr = phdr_to_first_uncached_entry(phdr); end = phdr_to_last_uncached_entry(phdr); cached = phdr_to_last_cached_entry(phdr); + if (WARN_ON((void *)end > p_end || (void *)cached > p_end)) + return -EINVAL; + while (hdr < end) { if (hdr->canary != SMEM_PRIVATE_CANARY) goto bad_canary; @@ -383,6 +388,9 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem, hdr = uncached_entry_next(hdr); } + if (WARN_ON((void *)hdr > p_end)) + return -EINVAL; + /* Check that we don't grow into the cached region */ alloc_size = sizeof(*hdr) + ALIGN(size, 8); if ((void *)hdr + alloc_size > cached) { @@ -501,6 +509,8 @@ static void *qcom_smem_get_global(struct qcom_smem *smem, struct smem_header *header; struct smem_region *region; struct smem_global_entry *entry; + u64 entry_offset; + u32 e_size; u32 aux_base; unsigned i; @@ -515,9 +525,13 @@ static void *qcom_smem_get_global(struct qcom_smem *smem, region = &smem->regions[i]; if ((u32)region->aux_base == aux_base || !aux_base) { + e_size = le32_to_cpu(entry->size); + entry_offset = le32_to_cpu(entry->offset); + if (size != NULL) - *size = le32_to_cpu(entry->size); - return region->virt_base + le32_to_cpu(entry->offset); + *size = e_size; + + return region->virt_base + entry_offset; } } @@ -531,8 +545,12 @@ static void *qcom_smem_get_private(struct qcom_smem *smem, { struct smem_private_entry *e, *end; struct smem_partition_header *phdr; + void *item_ptr, *p_end; + u32 padding_data; + u32 e_size; phdr = (struct smem_partition_header __force *)part->virt_base; + p_end = (void *)phdr + part->size; e = phdr_to_first_uncached_entry(phdr); end = phdr_to_last_uncached_entry(phdr); @@ -542,36 +560,65 @@ static void *qcom_smem_get_private(struct qcom_smem *smem, goto invalid_canary; if (le16_to_cpu(e->item) == item) { - if (size != NULL) - *size = le32_to_cpu(e->size) - - le16_to_cpu(e->padding_data); + if (size != NULL) { + e_size = le32_to_cpu(e->size); + padding_data = le16_to_cpu(e->padding_data); - return uncached_entry_to_item(e); + if (WARN_ON(e_size > part->size || padding_data > e_size)) + return ERR_PTR(-EINVAL); + + *size = e_size - padding_data; + } + + item_ptr = uncached_entry_to_item(e); + if (WARN_ON(item_ptr > p_end)) + return ERR_PTR(-EINVAL); + + return item_ptr; } e = uncached_entry_next(e); } + if (WARN_ON((void *)e > p_end)) + return ERR_PTR(-EINVAL); + /* Item was not found in the uncached list, search the cached list */ e = phdr_to_first_cached_entry(phdr, part->cacheline); end = phdr_to_last_cached_entry(phdr); + if (WARN_ON((void *)e < (void *)phdr || (void *)end > p_end)) + return ERR_PTR(-EINVAL); + while (e > end) { if (e->canary != SMEM_PRIVATE_CANARY) goto invalid_canary; if (le16_to_cpu(e->item) == item) { - if (size != NULL) - *size = le32_to_cpu(e->size) - - le16_to_cpu(e->padding_data); + if (size != NULL) { + e_size = le32_to_cpu(e->size); + padding_data = le16_to_cpu(e->padding_data); + + if (WARN_ON(e_size > part->size || padding_data > e_size)) + return ERR_PTR(-EINVAL); + + *size = e_size - padding_data; + } + + item_ptr = cached_entry_to_item(e); + if (WARN_ON(item_ptr < (void *)phdr)) + return ERR_PTR(-EINVAL); - return cached_entry_to_item(e); + return item_ptr; } e = cached_entry_next(e, part->cacheline); } + if (WARN_ON((void *)e < (void *)phdr)) + return ERR_PTR(-EINVAL); + return ERR_PTR(-ENOENT); invalid_canary: @@ -648,14 +695,23 @@ int qcom_smem_get_free_space(unsigned host) phdr = part->virt_base; ret = le32_to_cpu(phdr->offset_free_cached) - le32_to_cpu(phdr->offset_free_uncached); + + if (ret > le32_to_cpu(part->size)) + return -EINVAL; } else if (__smem->global_partition.virt_base) { part = &__smem->global_partition; phdr = part->virt_base; ret = le32_to_cpu(phdr->offset_free_cached) - le32_to_cpu(phdr->offset_free_uncached); + + if (ret > le32_to_cpu(part->size)) + return -EINVAL; } else { header = __smem->regions[0].virt_base; ret = le32_to_cpu(header->available); + + if (ret > __smem->regions[0].size) + return -EINVAL; } return ret; @@ -918,13 +974,12 @@ qcom_smem_enumerate_partitions(struct qcom_smem *smem, u16 local_host) static int qcom_smem_map_toc(struct qcom_smem *smem, struct smem_region *region) { u32 ptable_start; - int ret; /* map starting 4K for smem header */ - region->virt_base = devm_ioremap_wc(dev, region->aux_base, SZ_4K); + region->virt_base = devm_ioremap_wc(smem->dev, region->aux_base, SZ_4K); ptable_start = region->aux_base + region->size - SZ_4K; /* map last 4k for toc */ - smem->ptable = devm_ioremap_wc(dev, ptable_start, SZ_4K); + smem->ptable = devm_ioremap_wc(smem->dev, ptable_start, SZ_4K); if (!region->virt_base || !smem->ptable) return -ENOMEM;
Structures in shared memory that can be modified by remote processors may have untrusted values, they should be validated before use. Adding proper validation before using fields of shared structures. Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com> --- drivers/soc/qcom/smem.c | 81 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 13 deletions(-)