Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-3

Rule: The upstream commit ID must be specified with a separate line above the commit text.
Subject: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps
Link: https://lore.kernel.org/stable/20240920103950.3931497-1-pulehui%40huaweicloud.com

Please ignore this mail if the patch is not relevant for upstream.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for
devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory
accounting [0]. The commit cannot be independently backported to the
5.10 stable branch, otherwise the related memory when creating devmap
will be unrestricted and the associated bpf selftest map_ptr will fail.
Let's roll back to rlimit-based memory accounting mode for devmap and
re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check
on 32-bit arches") to the 5.10 stable branch.

Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0]
Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches")
Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 kernel/bpf/devmap.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 07b5edb2c70f..7eb1282edc8e 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -109,6 +109,8 @@ static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
 static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
 {
 	u32 valsize = attr->value_size;
+	u64 cost = 0;
+	int err;
 
 	/* check sanity of attributes. 2 value sizes supported:
 	 * 4 bytes: ifindex
@@ -136,11 +138,21 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
 			return -EINVAL;
 
 		dtab->n_buckets = roundup_pow_of_two(dtab->map.max_entries);
+		cost += (u64) sizeof(struct hlist_head) * dtab->n_buckets;
+	} else {
+		cost += (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
+	}
 
+	/* if map size is larger than memlock limit, reject it */
+	err = bpf_map_charge_init(&dtab->map.memory, cost);
+	if (err)
+		return -EINVAL;
+
+	if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
 		dtab->dev_index_head = dev_map_create_hash(dtab->n_buckets,
 							   dtab->map.numa_node);
 		if (!dtab->dev_index_head)
-			return -ENOMEM;
+			goto free_charge;
 
 		spin_lock_init(&dtab->index_lock);
 	} else {
@@ -148,10 +160,14 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
 						      sizeof(struct bpf_dtab_netdev *),
 						      dtab->map.numa_node);
 		if (!dtab->netdev_map)
-			return -ENOMEM;
+			goto free_charge;
 	}
 
 	return 0;
+
+free_charge:
+	bpf_map_charge_finish(&dtab->map.memory);
+	return -ENOMEM;
 }
 
 static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
-- 
2.34.1

Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[Pu Lehui]

On 2024/9/20 18:37, kernel test robot wrote:
> Hi,
> 
> Thanks for your patch.
> 
> FYI: kernel test robot notices the stable kernel rule is not satisfied.
> 
> The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-3
> 
> Rule: The upstream commit ID must be specified with a separate line above the commit text.
> Subject: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps
> Link: https://lore.kernel.org/stable/20240920103950.3931497-1-pulehui%40huaweicloud.com
> 
> Please ignore this mail if the patch is not relevant for upstream.
> 

This fix only involves 5.10, other versions are no problem.

Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[Greg KH]

On Mon, Sep 23, 2024 at 02:38:07PM +0800, Pu Lehui wrote:
> 
> 
> On 2024/9/20 18:37, kernel test robot wrote:
> > Hi,
> > 
> > Thanks for your patch.
> > 
> > FYI: kernel test robot notices the stable kernel rule is not satisfied.
> > 
> > The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-3
> > 
> > Rule: The upstream commit ID must be specified with a separate line above the commit text.
> > Subject: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps
> > Link: https://lore.kernel.org/stable/20240920103950.3931497-1-pulehui%40huaweicloud.com
> > 
> > Please ignore this mail if the patch is not relevant for upstream.
> > 
> 
> This fix only involves 5.10, other versions are no problem.

why?  If this is true, then you need to get the relevant bpf maintainers
to review and ack it.

thanks,

greg k-h

Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[Greg Kroah-Hartman]

On Fri, Sep 20, 2024 at 10:39:50AM +0000, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for
> devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory
> accounting [0]. The commit cannot be independently backported to the
> 5.10 stable branch, otherwise the related memory when creating devmap
> will be unrestricted and the associated bpf selftest map_ptr will fail.
> Let's roll back to rlimit-based memory accounting mode for devmap and
> re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check
> on 32-bit arches") to the 5.10 stable branch.
> 
> Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0]
> Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches")
> Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")

Should we just revert these changes instead?

thanks,

greg k-h

Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[Pu Lehui]

On 2024/9/27 15:56, Greg Kroah-Hartman wrote:
> On Fri, Sep 20, 2024 at 10:39:50AM +0000, Pu Lehui wrote:
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for
>> devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory
>> accounting [0]. The commit cannot be independently backported to the
>> 5.10 stable branch, otherwise the related memory when creating devmap
>> will be unrestricted and the associated bpf selftest map_ptr will fail.
>> Let's roll back to rlimit-based memory accounting mode for devmap and
>> re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check
>> on 32-bit arches") to the 5.10 stable branch.
>>
>> Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0]
>> Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches")
>> Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")
> 
> Should we just revert these changes instead?

Yes, Greg. My patch is to revert these two commits and re-adapt commit 
225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches").

Shall we need to split this patch into multiple patches?

> 
> thanks,
> 
> greg k-h

Re: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps

[Greg Kroah-Hartman]

On Fri, Sep 27, 2024 at 04:03:36PM +0800, Pu Lehui wrote:
> 
> 
> On 2024/9/27 15:56, Greg Kroah-Hartman wrote:
> > On Fri, Sep 20, 2024 at 10:39:50AM +0000, Pu Lehui wrote:
> > > From: Pu Lehui <pulehui@huawei.com>
> > > 
> > > Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for
> > > devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory
> > > accounting [0]. The commit cannot be independently backported to the
> > > 5.10 stable branch, otherwise the related memory when creating devmap
> > > will be unrestricted and the associated bpf selftest map_ptr will fail.
> > > Let's roll back to rlimit-based memory accounting mode for devmap and
> > > re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check
> > > on 32-bit arches") to the 5.10 stable branch.
> > > 
> > > Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0]
> > > Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches")
> > > Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")
> > 
> > Should we just revert these changes instead?
> 
> Yes, Greg. My patch is to revert these two commits and re-adapt commit
> 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches").
> 
> Shall we need to split this patch into multiple patches?

Yes, please do so.