[Devel] Re: [PATCH 2/3] Make sockets proper objhash objects and use checkpoint_obj() on them

[Matt Helsley]

On Tue, Aug 25, 2009 at 01:55:08PM -0400, Oren Laadan wrote:
> 
> 
> Dan Smith wrote:
> > OL> It's perhaps more accurate to s/most sockets/some sockets/. It may
> > OL> be more likely for a socket to be checkpointed as a peer of
> > OL> another process, or as the sender of an skb.
> > 
> > Um, how about "most of the time" ?  I definitely think that the
> > (overwhelmingly) common case is a pair of sockets each attached to a
> > file descriptor.
> > 
> > OL> Now that you made 'struct sock' a 1st class object, they deserve to
> > OL> enjoy 1st class treatment :p  That also means proper collect() method
> > OL> - probably starting with the f_op ...
> > 
> > Okay.
> > 
> > OL> I may be mistaken, but I suspect that the suggested implementation
> > OL> cannot limit the depth of recursive calls to checkpoint_obj(). For
> > OL> instance, consider a dgram socket that received data from another
> > OL> dgram socket, that received data from another dgram, ad infinitum.
> > 
> > At the very least, a single receive socket is limited in how many
> > skb's may be queued for it, which limits an attacker's ability to
> > reach the "ad infinitum" case, I'd say.  Do we need something more?
> 
> Multiple buffers adds iteration, and one level of recursion. I had in
> mind a slightly different scenario: instead of many buffers for one
> socket, many sockets "chained" -
> 
> Assume N sockets S_1...S_n, all dgram, none is connected. Each socket
> S_i send one packet to S_i+1.  Suppose you first checkpoint S_n, then
> you'll need to checkpoint S_n-1, for which you'll need to checkpoint
> S_n-2 etc.
> 
> > OL> I'm thinking about the two other use cases that I mentioned:
> > OL> "dangling" (not-referenced by a file) and "pending" (not yet
> > OL> accepted) sockets.
> > 
> > OL> In both cases (well, at least with "pending"), the 'struct sock'
> > OL> exist but the 'struct socket' does not exit until after the socket
> > OL> is attached to a file descriptor. IIRC, the lifespan of 'struct
> > OL> socket' is coupled to that of the referencing file.
> > 
> > OL> In that case, I guess it make more sense to leave the 'struct
> > OL> socket' related data within ckpt_hdr_file_socket.
> > 
> > Hmm, not by my reading.  From what I can tell, the accept operation
> 
> You are right: sock_init_data() sets it up, and I believe it is
> for the entire lifetime of the sock/socket.
> 
> >>> +		return ERR_PTR(PTR_ERR(sk));
> > 
> > OL> Nit: I vaguely recall some disapproval of such construct...
> > OL> How about '(struct file *) sk' ?
> > 
> > Casting it to the wrong type seems less desirable to me.  I was
> > following the lead of:
> > 
> >   % fgrep -r 'ERR_PTR(PTR_ERR' . | wc -l
> >   36
> 
> Yep. That settles it then :)

Hmm, OK. For some reason I thought that pattern only showed up in
checkpoint/*...

I still think it would be nice to see a macro specifically for this.
I can submit a patch for that myself though.

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