[Devel] Re: [PATCH 1/2] c/r: Add AF_UNIX support (v5)

[Serge E. Hallyn]

Quoting Oren Laadan (orenl at cs.columbia.edu):
> Dan Smith wrote:
> > +static int sock_read_buffer(struct ckpt_ctx *ctx,
> > +			    struct sock *sock,
> > +			    struct sk_buff **skb)
> > +{
> > +	struct ckpt_hdr h;
> > +	int ret = 0;
> > +	int len;
> > +
> > +	len = _ckpt_read_hdr_type(ctx, &h, CKPT_HDR_SOCKET_BUFFER);
> > +	if (len < 0)
> > +		return len;
> > +
> > +	if (len > SKB_MAX_ALLOC) {
> > +		ckpt_debug("Socket buffer too big (%i > %solu)",
> > +			   len, SKB_MAX_ALLOC);
> > +		return -ENOSPC;
> > +	}
> > +
> > +	*skb = sock_alloc_send_skb(sock, len, MSG_DONTWAIT, &ret);
> 
> I looked at the socket code again, and I suspect this is wrong.

...

> The problem stems from trying to imitate the network code instead
> of reusing it - for example, by really sending data from the source
> socket (or a dummy one, if original no longer exists) to the target
> socket.

That also caused you to skip a bunch of security_* calls (at
the least here, at the recv equivalent, do_sock_getname, and at your
bind at restore).

I don't think simply inserting them here is the right thing to do,
bc then as the main code changes this code is likely to fall out of
sync.  So like Oren says, I think you need to do more re-use of the
common code.  For the bind() case, for instance, write a common helper
used by both sys_bind() and your restart bind, which does the
security check and then calls sock->ops->bind().  It makes your
patchset a bit more intrusive, but easier to maintain.

-serge
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers