On Mon, Jul 29, 2024 at 10:54:50AM +0200, Alexander Lobakin wrote: > From: Simon Horman <ho...@kernel.org> > Date: Fri, 26 Jul 2024 17:09:54 +0100 > > > On Wed, Jul 24, 2024 at 03:40:22PM +0200, Alexander Lobakin wrote: > >> The second tagged commit introduced a UAF, as it removed restoring > >> q_vector->vport pointers after reinitializating the structures. > >> This is due to that all queue allocation functions are performed here > >> with the new temporary vport structure and those functions rewrite > >> the backpointers to the vport. Then, this new struct is freed and > >> the pointers start leading to nowhere. > > [...] > > >> err_reset: > >> - idpf_vport_queues_rel(new_vport); > >> + idpf_send_add_queues_msg(vport, vport->num_txq, vport->num_complq, > >> + vport->num_rxq, vport->num_bufq); > >> + > >> +err_open: > >> + if (current_state == __IDPF_VPORT_UP) > >> + idpf_vport_open(vport); > > > > Hi Alexander, > > > > Can the system end up in an odd state if this call to idpf_vport_open(), or > > the one above, fails. Likewise if the above call to > > idpf_send_add_queues_msg() fails. > > Adding the queues with the parameters that were before changing them > almost can't fail. But if any of these two fails, it really will be in > an odd state...
Thanks for the clarification, this is my concern. > Perhaps we need to do a more powerful reset then? Can we somehow tell > the kernel that in fact our iface is down, so that the user could try > to enable it manually once again? > Anyway, feels like a separate series or patch to -next, what do you think? Yes, sure. I agree that this patch improves things, and more extreme corner cases can be addressed separately. With the above in mind, I'm happy with this patch. Reviewed-by: Simon Horman <ho...@kernel.org>
