> dependencyOn 2021/4/15 11:35, zhudi wrote:
> > From: Di Zhu <zhud...@huawei.com>
> >
> > We encountered a crash: in the packet receiving process, we got an
> > illegal VLAN device address, but the VLAN device address saved in
> > vmcore is correct. After checking the code, we found a possible data
> > competition:
> > CPU 0: CPU 1:
> > (RCU read lock) (RTNL lock)
> > vlan_do_receive() register_vlan_dev()
> > vlan_find_dev()
> >
> > ->__vlan_group_get_device() ->vlan_group_prealloc_vid()
> >
> > In vlan_group_prealloc_vid(), We need to make sure that kzalloc is
> > executed before assigning a value to vlan devices array, otherwise we
>
> As my understanding, there is a dependency between calling kzalloc() and
> assigning the address(returned from kzalloc()) to vg->vlan_devices_arrays,
> CPU and compiler can see the dependency, why can't it handling the
> dependency before adding the smp_wmb()?
>
> See CONTROL DEPENDENCIES section in Documentation/memory-
> barriers.txt:
>
> However, stores are not speculated. This means that ordering -is- provided
> for load-store control dependencies, as in the following example:
>
> q = READ_ONCE(a);
> if (q) {
> WRITE_ONCE(b, 1);
> }
>
Maybe I didn't make it clear. This memory isolation is to ensure the order of
memset(object, 0, size) in kzalloc() operations and the subsequent array
assignment statements.
kzalloc()
->memset(object, 0, size)
smp_wmb()
vg->vlan_devices_arrays[pidx][vidx] = array;
Because __vlan_group_get_device() function depends on this order
>
>
> > may get a wrong address from the hardware cache on another cpu.
> >
> > So fix it by adding memory barrier instruction to ensure the order of
> > memory operations.
> >
> > Signed-off-by: Di Zhu <zhud...@huawei.com>
> > ---
> > net/8021q/vlan.c | 2 ++
> > net/8021q/vlan.h | 3 +++
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c index
> > 8b644113715e..4f541e05cd3f 100644
> > --- a/net/8021q/vlan.c
> > +++ b/net/8021q/vlan.c
> > @@ -71,6 +71,8 @@ static int vlan_group_prealloc_vid(struct vlan_group
> *vg,
> > if (array == NULL)
> > return -ENOBUFS;
> >
> > + smp_wmb();
> > +
> > vg->vlan_devices_arrays[pidx][vidx] = array;
> > return 0;
> > }
> > diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h index
> > 953405362795..7408fda084d3 100644
> > --- a/net/8021q/vlan.h
> > +++ b/net/8021q/vlan.h
> > @@ -57,6 +57,9 @@ static inline struct net_device
> > *__vlan_group_get_device(struct vlan_group *vg,
> >
> > array = vg->vlan_devices_arrays[pidx]
> > [vlan_id /
> VLAN_GROUP_ARRAY_PART_LEN];
> > +
> > + smp_rmb();
> > +
> > return array ? array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN] :
> NULL; }
> >
> >
