I changed the CC list from netdev to linux-nfs.
On Tue, Mar 07, 2023 at 02:37:07PM -0300, Thadeu Lima de Souza Cascardo wrote:
> When sock_alloc_file fails to allocate a file, it will call sock_release.
> __sys_socket_file should then not call sock_release again, otherwise there
> will be a double free.
>
It's just a one liner to make Smatch warn about these sorts of user
after free bugs.
{ "sock_release", PARAM_FREED, 0, "$" },
Smatch found a related bug in nfs.
net/sunrpc/svcsock.c:938 svc_tcp_accept() warn: 'newsock' was already freed.
net/sunrpc/svcsock.c:1633 svc_create_socket() warn: 'sock' was already freed.
net/sunrpc/svcsock.c:1555 svc_addsock() error: dereferencing freed memory 'so'
net/sunrpc/svcsock.c:1633 svc_create_socket() warn: passing freed memory 'sock'
net/sunrpc/svcsock.c:938 svc_tcp_accept() warn: passing freed memory 'newsock'
I guess the svc_setup_socket() function should free sock on every error
path? It seems kind of gnarly.
I fixed another sock_release() double free in xen that was simpler.
regards,
dan carpenter
> On May 3, 2023, at 7:31 AM, Dan Carpenter <[email protected]> wrote:
>
> I changed the CC list from netdev to linux-nfs.
>
> On Tue, Mar 07, 2023 at 02:37:07PM -0300, Thadeu Lima de Souza Cascardo wrote:
>> When sock_alloc_file fails to allocate a file, it will call sock_release.
>> __sys_socket_file should then not call sock_release again, otherwise there
>> will be a double free.
>>
>
> It's just a one liner to make Smatch warn about these sorts of user
> after free bugs.
>
> { "sock_release", PARAM_FREED, 0, "$" },
>
> Smatch found a related bug in nfs.
>
> net/sunrpc/svcsock.c:938 svc_tcp_accept() warn: 'newsock' was already freed.
> net/sunrpc/svcsock.c:1633 svc_create_socket() warn: 'sock' was already freed.
> net/sunrpc/svcsock.c:1555 svc_addsock() error: dereferencing freed memory 'so'
> net/sunrpc/svcsock.c:1633 svc_create_socket() warn: passing freed memory 'sock'
> net/sunrpc/svcsock.c:938 svc_tcp_accept() warn: passing freed memory 'newsock'
>
> I guess the svc_setup_socket() function should free sock on every error
> path? It seems kind of gnarly.
I'm looking at current upstream, and I don't see svc_setup_socket()
releasing @sock on any return path. What am I missing?
Does your tree have "SUNRPC: Ensure server-side sockets have a sock->file" ?
--
Chuck Lever