From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 82DC0C432C0 for ; Wed, 20 Nov 2019 08:44:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 522B4223CA for ; Wed, 20 Nov 2019 08:44:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="b/oa1PyC" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727777AbfKTIoR (ORCPT ); Wed, 20 Nov 2019 03:44:17 -0500 Received: from mail-lj1-f195.google.com ([209.85.208.195]:33544 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726687AbfKTIoQ (ORCPT ); Wed, 20 Nov 2019 03:44:16 -0500 Received: by mail-lj1-f195.google.com with SMTP id t5so26570803ljk.0 for ; Wed, 20 Nov 2019 00:44:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=8ROSzcPUvYBvRlmXFaGWzJIcmMtF/PR2QjtS3/HVi90=; b=b/oa1PyCIi8YuMQdpfXMPqesJGqAmR7hwaaj99yHVMOj30r0s5jfUCojw4PAushp1I oGKj2VZk7D7jDHmCwakyYIhrhmzYXmgjuFRWV6MNC1qNRfGZD6Fbfbz0ilKicGv0KfeS lttnzlsKpdSJ+7xjSaEOnoUyzAFO3NzDUp/106G2fPHckvkX2Aq/SQVTALyS2a9Y0okN EXZ6foRmmMHvAc+/3k/CgoggWtSTtpKcC5GoL+NMm0XaTfb5Tea/y6+iHdmZhB7C6LSb 9/TMJh1+NxdFMJrWfPJWZcHCIMG0K+5HVQv5IEbx+DZN6EnRhTGGjUv7ZV6aBsABZ2IO UGYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=8ROSzcPUvYBvRlmXFaGWzJIcmMtF/PR2QjtS3/HVi90=; b=FKYpxVMK5KdEnMEgtmF7G0SIFYm0vW+Es8t7lIbnsC5LyY+44bY59OLKABKlLza5XD u0ayAfwcuJuy8WVQ3BQBZP9h2q/zLAiEN+MqPSOuV27HVJuo1ip9QfX9m4SF4WmEdkPV LMlYRBtNtbKip//alDYGBXXHAFkQtHd1LQRlcFX8469sM3olsGc14eZuWoVnvdelTtT4 IViRlckJ+Z6AIg15D5DT+Lj2Xcuu8FVCl/ot6Gy91tXYN/nFz2V261o6/8PSWCPqvRfS CmEdywPXsf9CX6V/3CtiwFierWlK8+u3RUlE8rUiISV5+UIAofyfRUekVhmJieeogpGe fgiA== X-Gm-Message-State: APjAAAVFUgdQFpgEDXB+b0OBKxm1k4Ng3L+Bc+lyTXQpiYMkZghVG1OI zsYI1eju5KpLL+4aC/fiqT9JaAfE X-Google-Smtp-Source: APXvYqwHKkryXWF56MBA9ATz0gZVPfDSEUeE14l03AFRHegAewtUyLqL9t5CsnyxVaF1lWFC1Z0Psw== X-Received: by 2002:a2e:9ec3:: with SMTP id h3mr1638469ljk.203.1574239452728; Wed, 20 Nov 2019 00:44:12 -0800 (PST) Received: from [172.31.190.83] ([86.57.146.226]) by smtp.gmail.com with ESMTPSA id c193sm2327928lfd.48.2019.11.20.00.44.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Nov 2019 00:44:12 -0800 (PST) Subject: Re: io_uring: io_fail_links() should only consider first linked timeout To: Jens Axboe , io-uring@vger.kernel.org References: From: Pavel Begunkov Message-ID: Date: Wed, 20 Nov 2019 11:44:11 +0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: io-uring-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: io-uring@vger.kernel.org On 11/20/2019 1:33 AM, Jens Axboe wrote: > We currently clear the linked timeout field if we cancel such a timeout, > but we should only attempt to cancel if it's the first one we see. > Others should simply be freed like other requests, as they haven't > been started yet. > > Signed-off-by: Jens Axboe > > --- > > diff --git a/fs/io_uring.c b/fs/io_uring.c > index a79ef43367b1..d1085e4e8ae9 100644 > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -937,12 +937,12 @@ static void io_fail_links(struct io_kiocb *req) > if ((req->flags & REQ_F_LINK_TIMEOUT) && > link->submit.sqe->opcode == IORING_OP_LINK_TIMEOUT) { > io_link_cancel_timeout(link); > - req->flags &= ~REQ_F_LINK_TIMEOUT; > } else { > io_cqring_fill_event(link, -ECANCELED); > __io_double_put_req(link); > } > kfree(sqe_to_free); > + req->flags &= ~REQ_F_LINK_TIMEOUT; That's not necessary, but maybe would safer to keep. If REQ_F_LINK_TIMEOUT is set, than there was a link timeout request, and for it and only for it io_link_cancel_timeout() will be called. However, this is only true if linked timeout isn't fired. Otherwise, there is another bug, which isn't fixed by either of the patches. We need to clear REQ_F_LINK_TIMEOUT in io_link_timeout_fn() as well. Let: REQ -> L_TIMEOUT1 -> L_TIMEOUT2 1. L_TIMEOUT1 fired before REQ is completed 2. io_link_timeout_fn() removes L_TIMEOUT1 from the list: REQ|REQ_F_LINK_TIMEOUT -> L_TIMEOUT2 3. free_req(REQ) then call io_link_cancel_timeout(L_TIMEOUT2) leaking it (as described in my patch). P.S. haven't tried to test nor reproduce it yet. > } > > io_commit_cqring(ctx); > -- Pavel Begunkov