On Tue, Mar 28, 2017 at 10:00 AM, Andrey Konovalov <andreyk...@google.com> wrote: > When calculating rb->frames_per_block * req->tp_block_nr the result > can overflow. > > Add a check that tp_block_size * tp_block_nr <= UINT_MAX. > > Since frames_per_block <= tp_block_size, the expression would > never overflow. > > Signed-off-by: Andrey Konovalov <andreyk...@google.com> > --- > net/packet/af_packet.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 506348abdf2f..c5c43fff8c01 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -4197,6 +4197,9 @@ static int packet_set_ring(struct sock *sk, union > tpacket_req_u *req_u, > goto out; > if (unlikely(req->tp_frame_size == 0)) > goto out; > + if (unlikely((u64)req->tp_block_size * req->tp_block_nr > > + UINT_MAX)) > + goto out; So this may be pedantic, but really the only guarantee that you have for the 'unsigned int' type of these fields is that they are _at least_ 16 bits. There is no guarantee on the upper bound size, so casting to a u64 will be problematic on a compiler that happens to use 64 bits for 'unsigned int'. I'm not aware of any that use greater than 32 bits right now and using one that does may very well break other things in the kernel, but here we are... Perhaps a alternative fix would be to do the multiplication into an 'unsigned int' type and ensure that the result is larger than each of the original two values?
The real issue is that explicit size types should have been used in this userspace structure.
