From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id D076C3B2A4 for ; Thu, 9 Jan 2025 07:11:40 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736424700; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6UX0y6Cp3sQrCE2nh5veQWVG7OewsvHWIAkmGcNadoM=; b=afR7TU/SrO6VLHwoJlMf58aZbcmwtTa2XzZ8lMrPFsrkY+8B50vdhvaArWO5WFHKuAL0t7 oEYtK6HyHEczwsDvBkWnSFhthw2REmby8JyARPov8P1AhhiRPQdbW6y82ljOPycVkYqea4 k5lYyYPkQxaMbHOjUmbU6Pu3RgyUcgM= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-464-QVAmdHiCOtafciKL5FfqDQ-1; Thu, 09 Jan 2025 07:11:38 -0500 X-MC-Unique: QVAmdHiCOtafciKL5FfqDQ-1 X-Mimecast-MFC-AGG-ID: QVAmdHiCOtafciKL5FfqDQ Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-467be89d064so8756871cf.3 for ; Thu, 09 Jan 2025 04:11:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736424698; x=1737029498; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6UX0y6Cp3sQrCE2nh5veQWVG7OewsvHWIAkmGcNadoM=; b=gvJOzmYbWdElwSCwOsuct5l2gBZXlrxalt/+DXuOKL4An0appidR+7hxr2EaJxYPfu 9IQfYFDHm7YH/FFAZrA4REEFwv5qAQoz8eMBG6QCkH+JlISyn2BYsVGbAHhHFY7+1SAm WmSaEU1hlDjD7qK6+zF2JSLP7lXY8mSLH/6VILUlH2TKhq0o8r20vQzwr0IWWCfWm7Jg FjksKWXuwWV8H3H8k+Ebu2B0X7C6lCv0JUdArlEI4bcUemzo/6jUt9Dve9cedJldutwd yU/8rbfP9DVnDGPdNeVRHgASy4Ta8v/Ue2tnWCcU387MW8Doud66Rm+/Q4hcIuX4BewY Xn0w== X-Forwarded-Encrypted: i=1; AJvYcCUr2pkD2BBq2TET0xncPC6Fz5T8Phj9l5gmHMoxN1pSF0iCywJE0WY1u+R9fkhzBXF7D0l7@lists.bufferbloat.net X-Gm-Message-State: AOJu0YyH/TDmO9DzpxT1/HcO1XxCJZWr6gKg4n1YHZI06fnSpEvN/xGf N1pgeMw5HYErreYwu4iHT9tBxpf+/8C/cJgPeoZQMbzNxrbplrKoSWeWVCFxh+7weFy9LWANkmf dDRXB2Byz7s62BaKWTo6vuQ2tpKfsyG86fK50bMc9eEMNsp4hW30Y0NY+e0U= X-Gm-Gg: ASbGncsVHPxril9x4/T0ksajQ0QRlpZng/E2QTqoGxb1jzcRdHW4wqkAzxSwIErFe+l WF2HinwPIcfaixsfmGDe/9Gern+nQnWDxsfKuA2uDZIZY2XB6EAlpSH/eJklRYQaSUgCRyVLRb8 n4oUfhC21nWEAKki5YaGBGqzbyJOwH+ZRu2Wyx+STeSCY93B8aoowlQIdvTV+b4zM2euHlGnERq jlxcEGbtI6RFF0L6OkBIZlLSaf9b3qgO+UFR5MqHWA++Xofco2SAedvZbirabF7PQ5rQ6B8zCJZ rgtAKehB X-Received: by 2002:a05:622a:242:b0:467:54e5:ceaa with SMTP id d75a77b69052e-46c71079ea3mr95295191cf.9.1736424698328; Thu, 09 Jan 2025 04:11:38 -0800 (PST) X-Google-Smtp-Source: AGHT+IGqKaxlWIfd8ANpdNqMYSDWxka7BN+0vCp0a2UKun8ljcvGi8/js/ptgg8silyjum2+0LQEyA== X-Received: by 2002:a05:622a:242:b0:467:54e5:ceaa with SMTP id d75a77b69052e-46c71079ea3mr95294811cf.9.1736424697984; Thu, 09 Jan 2025 04:11:37 -0800 (PST) Received: from [192.168.88.253] (146-241-2-244.dyn.eolo.it. [146.241.2.244]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-46a3eb30dc4sm206077481cf.76.2025.01.09.04.11.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Jan 2025 04:11:37 -0800 (PST) Message-ID: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Jamal Hadi Salim , Cong Wang , Jiri Pirko Cc: syzbot+f63600d288bfb7057424@syzkaller.appspotmail.com, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Simon Horman , cake@lists.bufferbloat.net, netdev@vger.kernel.org References: <20250107120105.70685-1-toke@redhat.com> From: Paolo Abeni In-Reply-To: <20250107120105.70685-1-toke@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: RjXIIQcY0EKTv4ZcUPgwMn-dK9oHTvY9OVDGlapIXzw_1736424698 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Fri, 06 Jun 2025 06:40:58 -0400 Subject: Re: [Cake] [PATCH net v2] sched: sch_cake: add bounds checks to host bulk flow fairness counts X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Thu, 09 Jan 2025 12:11:40 -0000 X-Original-Date: Thu, 9 Jan 2025 13:11:34 +0100 X-List-Received-Date: Thu, 09 Jan 2025 12:11:40 -0000 On 1/7/25 1:01 PM, Toke Høiland-Jørgensen wrote: > Even though we fixed a logic error in the commit cited below, syzbot > still managed to trigger an underflow of the per-host bulk flow > counters, leading to an out of bounds memory access. > > To avoid any such logic errors causing out of bounds memory accesses, > this commit factors out all accesses to the per-host bulk flow counters > to a series of helpers that perform bounds-checking before any > increments and decrements. This also has the benefit of improving > readability by moving the conditional checks for the flow mode into > these helpers, instead of having them spread out throughout the > code (which was the cause of the original logic error). > > v2: > - Remove now-unused srchost and dsthost local variables in cake_dequeue() Small nit: the changelog should come after the '---' separator. No need to repost just for this. > Fixes: 546ea84d07e3 ("sched: sch_cake: fix bulk flow accounting logic for host fairness") > Reported-by: syzbot+f63600d288bfb7057424@syzkaller.appspotmail.com > Signed-off-by: Toke Høiland-Jørgensen > --- > net/sched/sch_cake.c | 140 +++++++++++++++++++++++-------------------- > 1 file changed, 75 insertions(+), 65 deletions(-) > > diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c > index 8d8b2db4653c..2c2e2a67f3b2 100644 > --- a/net/sched/sch_cake.c > +++ b/net/sched/sch_cake.c > @@ -627,6 +627,63 @@ static bool cake_ddst(int flow_mode) > return (flow_mode & CAKE_FLOW_DUAL_DST) == CAKE_FLOW_DUAL_DST; > } > > +static void cake_dec_srchost_bulk_flow_count(struct cake_tin_data *q, > + struct cake_flow *flow, > + int flow_mode) > +{ > + if (likely(cake_dsrc(flow_mode) && > + q->hosts[flow->srchost].srchost_bulk_flow_count)) > + q->hosts[flow->srchost].srchost_bulk_flow_count--; > +} > + > +static void cake_inc_srchost_bulk_flow_count(struct cake_tin_data *q, > + struct cake_flow *flow, > + int flow_mode) > +{ > + if (likely(cake_dsrc(flow_mode) && > + q->hosts[flow->srchost].srchost_bulk_flow_count < CAKE_QUEUES)) > + q->hosts[flow->srchost].srchost_bulk_flow_count++; > +} > + > +static void cake_dec_dsthost_bulk_flow_count(struct cake_tin_data *q, > + struct cake_flow *flow, > + int flow_mode) > +{ > + if (likely(cake_ddst(flow_mode) && > + q->hosts[flow->dsthost].dsthost_bulk_flow_count)) > + q->hosts[flow->dsthost].dsthost_bulk_flow_count--; > +} > + > +static void cake_inc_dsthost_bulk_flow_count(struct cake_tin_data *q, > + struct cake_flow *flow, > + int flow_mode) > +{ > + if (likely(cake_ddst(flow_mode) && > + q->hosts[flow->dsthost].dsthost_bulk_flow_count < CAKE_QUEUES)) > + q->hosts[flow->dsthost].dsthost_bulk_flow_count++; > +} > + > +static u16 cake_get_flow_quantum(struct cake_tin_data *q, > + struct cake_flow *flow, > + int flow_mode) > +{ > + u16 host_load = 1; > + > + if (cake_dsrc(flow_mode)) > + host_load = max(host_load, > + q->hosts[flow->srchost].srchost_bulk_flow_count); > + > + if (cake_ddst(flow_mode)) > + host_load = max(host_load, > + q->hosts[flow->dsthost].dsthost_bulk_flow_count); > + > + /* The get_random_u16() is a way to apply dithering to avoid > + * accumulating roundoff errors > + */ > + return (q->flow_quantum * quantum_div[host_load] + > + get_random_u16()) >> 16; dithering is now applied on both enqueue and dequeue, while prior to this patch it only happened on dequeue. Is that intentional? can't lead to (small) flow_deficit increase? Thanks! Paolo