0

I'm writing a x86_64 kernel module targeting Linux v3-v4 which uses set_memory_rw() on an address of a kernel symbol. While the call works I'm getting a warning without any explanation:

[  596.183643] ------------[ cut here ]------------
[  596.183667] WARNING: at arch/x86/mm/pageattr.c:962 change_page_attr_set_clr+0x2ca/0x450()

Previously I was using a different method which called virt_to_page() and then set_pages_rw(). However as the kernel source states this is a deprecated API and it recommends using set_memory_rw() instead

These APIs should be considered deprecated and are likely going to be removed in the future. [...] Specifically, many users of the old APIs had a virtual address, called virt_to_page() or vmalloc_to_page() on that address to get a struct page* that the old API required. To convert these cases, use set_memory_*() on the original virtual address, do not use these functions.

Can anyone explain what is the problem here?
I think I'm tripping this check, which suggests I can just add *addr &= PAGE_MASK; but I don't want to do it blindly.

kiler129
  • 1,063
  • 2
  • 11
  • 21
  • "I'm getting a warning without any explanation" - Explanations could be in the code the warning message refers to. In you case, it is `arch/x86/mm/pageattr.c:962` line which you need to check. Even if there is no explanations, the line should contain a failed check, which could give you a hint what is going wrong. Note, that a line of code is specific to your Linux kernel version and patches. E.g. version 3.10.108 doesn't contain a warning at given line: https://elixir.bootlin.com/linux/v3.10.108/source/arch/x86/mm/pageattr.c#L962 – Tsyvarev Jun 27 '21 at 07:04
  • @Tsyvarev in my case line 962 is equal to the standard 941: https://elixir.bootlin.com/linux/v3.10.108/source/arch/x86/mm/pageattr.c#L941 - while I see the check I don't clearly understand where is the problem... does it force the address to point to the beginning of the page? – kiler129 Jun 27 '21 at 08:16
  • "does it force the address to point to the beginning of the page?" - Yes, it needs the address to be page-aligned. Whatever architecture you use (x86, arm, etc.), it cannot set a *particular* address to be writable or readable. The minimum unit of memory writability or readability is a **page**. – Tsyvarev Jun 27 '21 at 08:40

1 Answers1

2

I think I'm tripping this check, which suggests I can just add *addr &= PAGE_MASK; but I don't want to do it blindly.

Sounds good to me (it's extremely likely that it is an "address not aligned to page boundary" problem; and it's extremely likely that not wanting to do it blindly is a good idea).

The likely case is that you have a range of addresses that you care about (and not a range of pages); and that is how you ended up with this problem. This implies:

a) You'll have be careful when calculating numpages (e.g. the original range could be like "4 bytes, with 2 bytes at the end of one page and 2 bytes at the start of the next page"). The right way is to round the "starting address" down (pg = start_address & PAGE_MASK;), round the "ending address" up (end_page = ((start_address + size_in_bytes - 1) | ~PAGE_MASK) + 1;), then calculate the number of pages (numpages = end_page - pg).

b) There may be other things in the same page/s that you do not want using the new page attributes. If that is the case then you've got a fundamental design problem.

Brendan
  • 35,656
  • 2
  • 39
  • 66
  • Thank you! That was an excellent explanation. Now I know where's the issue and how to fix that - while I didn't find any instances of functions crossing the page boundary in my case it can definitely happen. – kiler129 Jun 28 '21 at 20:32