| Message ID | 20231211141929.74027-1-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-fixes] riscv: Fix wrong usage of lm_alias() when splitting a huge linear mapping | expand |
On Mon, Dec 11, 2023 at 03:19:29PM +0100, Alexandre Ghiti wrote: > lm_alias() can only be used on kernel mappings since it explicitly uses > __pa_symbol(), so simply fix this by checking where the address belongs > to before. > > Fixes: 311cd2f6e253 ("riscv: Fix set_memory_XX() and set_direct_map_XX() by splitting huge linear mappings") > Reported-by: syzbot+afb726d49f84c8d95ee1@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/linux-riscv/000000000000620dd0060c02c5e1@google.com/ > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> ../arch/riscv/mm/pageattr.c:311:14: warning: variable 'ret' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized] > --- > arch/riscv/mm/pageattr.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c > index fc5fc4f785c4..f5f8aa1d38d6 100644 > --- a/arch/riscv/mm/pageattr.c > +++ b/arch/riscv/mm/pageattr.c > @@ -304,9 +304,16 @@ static int __set_memory(unsigned long addr, int numpages, pgprot_t set_mask, > if (ret) > goto unlock; > } > - } else if (is_kernel_mapping(start) || is_linear_mapping(start)) { > - lm_start = (unsigned long)lm_alias(start); > - lm_end = (unsigned long)lm_alias(end); > + } else { > + if (is_kernel_mapping(start)) { > + lm_start = (unsigned long)lm_alias(start); > + lm_end = (unsigned long)lm_alias(end); > + } else if (is_linear_mapping(start)) { > + lm_start = start; > + lm_end = end; > + } else { > + goto unlock; > + } > > ret = split_linear_mapping(lm_start, lm_end); > if (ret) > -- > 2.39.2 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Alexandre, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on v6.7-rc5 next-20231211] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Alexandre-Ghiti/riscv-Fix-wrong-usage-of-lm_alias-when-splitting-a-huge-linear-mapping/20231211-222156 base: linus/master patch link: https://lore.kernel.org/r/20231211141929.74027-1-alexghiti%40rivosinc.com patch subject: [PATCH -fixes] riscv: Fix wrong usage of lm_alias() when splitting a huge linear mapping config: riscv-alldefconfig (https://download.01.org/0day-ci/archive/20231212/202312120825.UkTJCa1g-lkp@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231212/202312120825.UkTJCa1g-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312120825.UkTJCa1g-lkp@intel.com/ All warnings (new ones prefixed by >>): >> arch/riscv/mm/pageattr.c:311:14: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 311 | } else if (is_linear_mapping(start)) { | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/page.h:130:2: note: expanded from macro 'is_linear_mapping' 130 | ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/mm/pageattr.c:348:9: note: uninitialized use occurs here 348 | return ret; | ^~~ arch/riscv/mm/pageattr.c:311:10: note: remove the 'if' if its condition is always true 311 | } else if (is_linear_mapping(start)) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 312 | lm_start = start; 313 | lm_end = end; 314 | } else { | ~~~~~~~ 315 | goto unlock; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 316 | } | ~~~~~~~~~~~~~~~~~ >> arch/riscv/mm/pageattr.c:311:14: warning: variable 'ret' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized] 311 | } else if (is_linear_mapping(start)) { | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/page.h:130:3: note: expanded from macro 'is_linear_mapping' 130 | ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE)) | ^~~~~~~~~~~~~~~~~~ arch/riscv/mm/pageattr.c:348:9: note: uninitialized use occurs here 348 | return ret; | ^~~ arch/riscv/mm/pageattr.c:311:14: note: remove the '&&' if its condition is always true 311 | } else if (is_linear_mapping(start)) { | ^ arch/riscv/include/asm/page.h:130:3: note: expanded from macro 'is_linear_mapping' 130 | ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE)) | ^ arch/riscv/mm/pageattr.c:265:9: note: initialize the variable 'ret' to silence this warning 265 | int ret; | ^ | = 0 2 warnings generated. vim +311 arch/riscv/mm/pageattr.c 261 262 static int __set_memory(unsigned long addr, int numpages, pgprot_t set_mask, 263 pgprot_t clear_mask) 264 { 265 int ret; 266 unsigned long start = addr; 267 unsigned long end = start + PAGE_SIZE * numpages; 268 unsigned long __maybe_unused lm_start; 269 unsigned long __maybe_unused lm_end; 270 struct pageattr_masks masks = { 271 .set_mask = set_mask, 272 .clear_mask = clear_mask 273 }; 274 275 if (!numpages) 276 return 0; 277 278 mmap_write_lock(&init_mm); 279 280 #ifdef CONFIG_64BIT 281 /* 282 * We are about to change the permissions of a kernel mapping, we must 283 * apply the same changes to its linear mapping alias, which may imply 284 * splitting a huge mapping. 285 */ 286 287 if (is_vmalloc_or_module_addr((void *)start)) { 288 struct vm_struct *area = NULL; 289 int i, page_start; 290 291 area = find_vm_area((void *)start); 292 page_start = (start - (unsigned long)area->addr) >> PAGE_SHIFT; 293 294 for (i = page_start; i < page_start + numpages; ++i) { 295 lm_start = (unsigned long)page_address(area->pages[i]); 296 lm_end = lm_start + PAGE_SIZE; 297 298 ret = split_linear_mapping(lm_start, lm_end); 299 if (ret) 300 goto unlock; 301 302 ret = walk_page_range_novma(&init_mm, lm_start, lm_end, 303 &pageattr_ops, NULL, &masks); 304 if (ret) 305 goto unlock; 306 } 307 } else { 308 if (is_kernel_mapping(start)) { 309 lm_start = (unsigned long)lm_alias(start); 310 lm_end = (unsigned long)lm_alias(end); > 311 } else if (is_linear_mapping(start)) { 312 lm_start = start; 313 lm_end = end; 314 } else { 315 goto unlock; 316 } 317 318 ret = split_linear_mapping(lm_start, lm_end); 319 if (ret) 320 goto unlock; 321 322 ret = walk_page_range_novma(&init_mm, lm_start, lm_end, 323 &pageattr_ops, NULL, &masks); 324 if (ret) 325 goto unlock; 326 } 327 328 ret = walk_page_range_novma(&init_mm, start, end, &pageattr_ops, NULL, 329 &masks); 330 331 unlock: 332 mmap_write_unlock(&init_mm); 333 334 /* 335 * We can't use flush_tlb_kernel_range() here as we may have split a 336 * hugepage that is larger than that, so let's flush everything. 337 */ 338 flush_tlb_all(); 339 #else 340 ret = walk_page_range_novma(&init_mm, start, end, &pageattr_ops, NULL, 341 &masks); 342 343 mmap_write_unlock(&init_mm); 344 345 flush_tlb_kernel_range(start, end); 346 #endif 347 348 return ret; 349 } 350
diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c index fc5fc4f785c4..f5f8aa1d38d6 100644 --- a/arch/riscv/mm/pageattr.c +++ b/arch/riscv/mm/pageattr.c @@ -304,9 +304,16 @@ static int __set_memory(unsigned long addr, int numpages, pgprot_t set_mask, if (ret) goto unlock; } - } else if (is_kernel_mapping(start) || is_linear_mapping(start)) { - lm_start = (unsigned long)lm_alias(start); - lm_end = (unsigned long)lm_alias(end); + } else { + if (is_kernel_mapping(start)) { + lm_start = (unsigned long)lm_alias(start); + lm_end = (unsigned long)lm_alias(end); + } else if (is_linear_mapping(start)) { + lm_start = start; + lm_end = end; + } else { + goto unlock; + } ret = split_linear_mapping(lm_start, lm_end); if (ret)
lm_alias() can only be used on kernel mappings since it explicitly uses __pa_symbol(), so simply fix this by checking where the address belongs to before. Fixes: 311cd2f6e253 ("riscv: Fix set_memory_XX() and set_direct_map_XX() by splitting huge linear mappings") Reported-by: syzbot+afb726d49f84c8d95ee1@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-riscv/000000000000620dd0060c02c5e1@google.com/ Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/mm/pageattr.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)