>> diff --git a/net/can/raw.c b/net/can/raw.c
>> index ed4fcb7ab0c3..cd5a49380116 100644
>> --- a/net/can/raw.c
>> +++ b/net/can/raw.c
>> @@ -546,10 +546,18 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>> return -EFAULT;
>> }
>> + rtnl_lock();
>> lock_sock(sk);
>> - if (ro->bound && ro->ifindex)
>> + if (ro->bound && ro->ifindex) {
>> dev = dev_get_by_index(sock_net(sk), ro->ifindex);
>> + if (!dev) {
>
>
>> + if (count > 1)
>> + kfree(filter);
>
> This was NOT suggested!
>
> I've been talking about removing the other kfree() "improvement" you suggested.
>
> The kfree() should only be done when ro->bound and ro->ifindex are cleared.
>
> So when you remove these two lines it should be ok.
>
> Please try to increase the context in the diff.
>
> Thanks,
> Oliver
Sorry, I am a little confused.
The following codes are the latest raw_setsockopt function realization(ignore some non-key parts)
with my patch. Now we assume the condition that count more than 1, ro->bound and ro->ifindex
are not zero, dev_get_by_index() will return NULL. We analyze the code logic.
static int raw_setsockopt(struct socket *sock, int level, int optname,
sockptr_t optval, unsigned int optlen)
{
......
struct can_filter *filter = NULL;
......
switch (optname) {
case CAN_RAW_FILTER:
......
if (count > 1) {
/* filter does not fit into dfilter => alloc space */
filter = memdup_sockptr(optval, optlen); // filter point to a heap memory
if (IS_ERR(filter))
return PTR_ERR(filter);
} else if (count == 1) {
......
}
rtnl_lock();
lock_sock(sk);
if (ro->bound && ro->ifindex) {
dev = dev_get_by_index(sock_net(sk), ro->ifindex);
/*
* dev == NULL is exception. The function will exit abnormally.
* Memory pointed by filer does not forward to anyone for maintenance.
* If we do not kfree(filter) here, memory will be leaked after function exit.
*/
if (!dev) {
if (count > 1)
kfree(filter);
err = -ENODEV;
goto out_fil;
}
}
if (ro->bound) {
/* (try to) register the new filters */
if (count == 1)
err = raw_enable_filters(sock_net(sk), dev, sk,
&sfilter, 1);
else
err = raw_enable_filters(sock_net(sk), dev, sk,
filter, count);
if (err) {
if (count > 1)
kfree(filter);
goto out_fil;
}
/* remove old filter registrations */
raw_disable_filters(sock_net(sk), dev, sk, ro->filter,
ro->count);
}
/* remove old filter space */
if (ro->count > 1)
kfree(ro->filter);
/* link new filters to the socket */
if (count == 1) {
/* copy filter data for single filter */
ro->dfilter = sfilter;
filter = &ro->dfilter;
}
ro->filter = filter;
ro->count = count;
out_fil:
if (dev)
dev_put(dev);
release_sock(sk);
rtnl_unlock();
break;
......
return err;
}
So I think my modification is right. Thank you.
