Noticing this patch only here rather than the original post, sorry. On Sun, Jul 16, 2023 at 09:44:32PM +0200, Greg Kroah-Hartman wrote: > From: David Howells <dhowells@xxxxxxxxxx> > ... > [ Upstream commit 86d7bd6e66e9925f0f04a7bcf3c92c05fdfefb5a ] > - o2net_hand = kzalloc(sizeof(struct o2net_handshake), GFP_KERNEL); > - o2net_keep_req = kzalloc(sizeof(struct o2net_msg), GFP_KERNEL); > - o2net_keep_resp = kzalloc(sizeof(struct o2net_msg), GFP_KERNEL); > - if (!o2net_hand || !o2net_keep_req || !o2net_keep_resp) > + folio = folio_alloc(GFP_KERNEL | __GFP_ZERO, 0); > + if (!folio) > goto out; > > + p = folio_address(folio); > + o2net_hand = p; > + p += sizeof(struct o2net_handshake); > + o2net_keep_req = p; > + p += sizeof(struct o2net_msg); > + o2net_keep_resp = p; Do we care that we aren't validating sizes here? The original code allocates the structures by size. This code grabs an order 0 folio and assumes the total size of these structures is smaller than a page. But while we "know" today that the handshake messages have no payload, `o2net_msg.buf` could theoretically be filled by more data. What if someone decided to change the code to put some payload in `o2net_keep_resp.buf`? Yes, we would hope they would think to add the appropriate allocation. But should we be validating this with some kind of safety check? Thanks, Joel -- "I almost ran over an angel He had a nice big fat cigar. 'In a sense,' he said, 'You're alone here So if you jump, you'd best jump far.'" http://www.jlbec.org/ jlbec@xxxxxxxxxxxx
