Re: [Qemu-devel] [PULL V2 24/26] net: ignore packet size greater than INT_MAX

[Dima Stepanov]

On Thu, Nov 15, 2018 at 10:47:04AM +0800, Jason Wang wrote:
> 
> On 2018/11/15 上午12:23, Dima Stepanov wrote:
> >On Wed, Nov 14, 2018 at 10:59:32AM +0800, Jason Wang wrote:
> >>On 2018/11/13 下午11:41, Dima Stepanov wrote:
> >>>Hi Jason,
> >>>
> >>>I know that this patch has been already merged to stable, but i have a
> >>>question:
> >>>
> >>>On Fri, Oct 19, 2018 at 11:22:23AM +0800, Jason Wang wrote:
> >>>>There should not be a reason for passing a packet size greater than
> >>>>INT_MAX. It's usually a hint of bug somewhere, so ignore packet size
> >>>>greater than INT_MAX in qemu_deliver_packet_iov()
> >>>>
> >>>>CC:address@hidden
> >>>>Reported-by: Daniel Shapira<address@hidden>
> >>>>Reviewed-by: Michael S. Tsirkin<address@hidden>
> >>>>Signed-off-by: Jason Wang<address@hidden>
> >>>>---
> >>>>  net/net.c | 7 ++++++-
> >>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/net/net.c b/net/net.c
> >>>>index c66847e..07c194a 100644
> >>>>--- a/net/net.c
> >>>>+++ b/net/net.c
> >>>>@@ -712,10 +712,15 @@ ssize_t qemu_deliver_packet_iov(NetClientState 
> >>>>*sender,
> >>>>                                  void *opaque)
> >>>>  {
> >>>>      NetClientState *nc = opaque;
> >>>>+    size_t size = iov_size(iov, iovcnt);
> >>>>      int ret;
> >>>>+    if (size > INT_MAX) {
> >>>>+        return size;
> >>>Is it okay that the function returns ssize_t (signed), but the type of the
> >>>size variable is size_t (unsigned)? For now the top level routine checks
> >>>the return value only for 0, but anyway we can return negative value
> >>>here instead of positive. What do you think?
> >>>
> >>>Regards, Dima.
> >>>
> >>Any non zero value should be ok here. Actually I think because of the
> >>conversion from size_t to ssize_t, caller actually see negative value?
> >I believe it depends. If long (ssize_t and size_t type) is 8 bytes, then
> >the routine can sometimes return positive values and sometimes negative.
> >I fully agree that in the current case any non zero value should be
> >okay. I just wanted to point on the inconsistency in types and as a
> >result a return value.
> 
> 
> I see, want to post a patch for this?
> 
> Thanks

Yes, will take a look into it and prepare a patch.

Thanks, Dima.
> 
> 
> >Dima.
> >>Thanks
> >>