From: Kairui Song kasong@tencent.com
The order check and fallback loop is updating the index value on every loop, this will cause the index to be aligned by a larger value while the loop shrinks the order.
This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].
Cc: stable@vger.kernel.org Link: https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy... [1] Fixes: e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem") Signed-off-by: Kairui Song kasong@tencent.com ---
Changes from V1: - Link to V1: https://lore.kernel.org/linux-mm/20251021190436.81682-1-ryncsn@gmail.com/ - Remove unnecessary cleanup and simplify the commit message.
mm/shmem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c index b50ce7dbc84a..7559773ebb30 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1895,10 +1895,11 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, order = highest_order(suitable_orders); while (suitable_orders) { pages = 1UL << order; - index = round_down(index, pages); - folio = shmem_alloc_folio(gfp, order, info, index); - if (folio) + folio = shmem_alloc_folio(gfp, order, info, round_down(index, pages)); + if (folio) { + index = round_down(index, pages); goto allocated; + }
if (pages == HPAGE_PMD_NR) count_vm_event(THP_FILE_FALLBACK);
diff --git a/mm/shmem.c b/mm/shmem.c index b50ce7dbc84a..7559773ebb30 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1895,10 +1895,11 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, order = highest_order(suitable_orders); while (suitable_orders) { pages = 1UL << order;
index = round_down(index, pages);folio = shmem_alloc_folio(gfp, order, info, index);if (folio)
folio = shmem_alloc_folio(gfp, order, info, round_down(index, pages));if (folio) {index = round_down(index, pages); goto allocated;}
Could this be a temporary variable to store round_down(index, pages)?
if (pages == HPAGE_PMD_NR) count_vm_event(THP_FILE_FALLBACK);--
Thanks Barry
On Thu, Oct 23, 2025 at 1:51 PM Barry Song 21cnbao@gmail.com wrote:
diff --git a/mm/shmem.c b/mm/shmem.c index b50ce7dbc84a..7559773ebb30 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1895,10 +1895,11 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, order = highest_order(suitable_orders); while (suitable_orders) { pages = 1UL << order;
index = round_down(index, pages);folio = shmem_alloc_folio(gfp, order, info, index);if (folio)
folio = shmem_alloc_folio(gfp, order, info, round_down(index, pages));if (folio) {index = round_down(index, pages); goto allocated;}Could this be a temporary variable to store round_down(index, pages)?
Right we can do that, but the generated code should be the same, the compiler is smart enough, I just checked the generated code with gcc / clang.
Do you think the code will be cleaner with a temporary variable? I can send a V3 if anyone suggests, it's really a trivial change.
On Thu, Oct 23, 2025 at 6:58 PM Kairui Song ryncsn@gmail.com wrote:
On Thu, Oct 23, 2025 at 1:51 PM Barry Song 21cnbao@gmail.com wrote:
diff --git a/mm/shmem.c b/mm/shmem.c index b50ce7dbc84a..7559773ebb30 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1895,10 +1895,11 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, order = highest_order(suitable_orders); while (suitable_orders) { pages = 1UL << order;
index = round_down(index, pages);folio = shmem_alloc_folio(gfp, order, info, index);if (folio)
folio = shmem_alloc_folio(gfp, order, info, round_down(index, pages));if (folio) {index = round_down(index, pages); goto allocated;}Could this be a temporary variable to store round_down(index, pages)?
Right we can do that, but the generated code should be the same, the compiler is smart enough, I just checked the generated code with gcc / clang.
Do you think the code will be cleaner with a temporary variable? I can send a V3 if anyone suggests, it's really a trivial change.
Personally, I think a temporary variable makes the code cleaner, but I don’t have any strong preference.
The fix looks correct to me.
Thanks Barry
linux-stable-mirror@lists.linaro.org
-
Barry Song -
Kairui Song