On Thu, 26 Nov 2020 17:22:08 +0300 Denis Kirjanov wrote:
> On 11/26/20, Jakub Kicinski <k...@kernel.org> wrote:
> > On Tue, 24 Nov 2020 15:24:21 +0300 Denis Kirjanov wrote:
> >> in the case of the socket which is bound to an adress
> >> there is no sense to create a path in the next attempts
> >
> >> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> >> index 41c3303c3357..fd76a8fe3907 100644
> >> --- a/net/unix/af_unix.c
> >> +++ b/net/unix/af_unix.c
> >> @@ -1021,7 +1021,7 @@ static int unix_bind(struct socket *sock, struct
> >> sockaddr *uaddr, int addr_len)
> >>
> >> err = -EINVAL;
> >> if (addr_len < offsetofend(struct sockaddr_un, sun_family) ||
> >> - sunaddr->sun_family != AF_UNIX)
> >> + sunaddr->sun_family != AF_UNIX || u->addr)
> >> goto out;
> >>
> >> if (addr_len == sizeof(short)) {
> >> @@ -1049,10 +1049,6 @@ static int unix_bind(struct socket *sock, struct
> >> sockaddr *uaddr, int addr_len)
> >> if (err)
> >> goto out_put;
> >>
> >> - err = -EINVAL;
> >> - if (u->addr)
> >> - goto out_up;
> >> -
> >> err = -ENOMEM;
> >> addr = kmalloc(sizeof(*addr)+addr_len, GFP_KERNEL);
> >> if (!addr)
> >
> > Well, after your change the check on u->addr is no longer protected by
> > u->bindlock. Is that okay?
>
> Since we're just checking the assigned address and it's an atomic
> operation I think it's okay.
The access to the variable may be atomic, but what protects two
concurrent binds() from progressing past the check and binding to
different paths?
I don't know this code at all, but looks to me like the pattern is
basically:
lock()
if (obj->thing)
goto err; /* already bound to a thing */
thing = alloc()
setup_thing(thing);
obj->thing = thing;
err:
unlock()
> A process performing binding is still protected.
Isn't checking "did someone already bind" not part of the process of
binding?
