Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 21 Sep 2022 14:51:38 -0400
From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Steffen Klassert <steffen.klassert@secunet.com>
Cc:     eadavis@sina.com, linux-crypto@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        syzbot+bc05445bc14148d51915@syzkaller.appspotmail.com,
        syzkaller-bugs@googlegroups.com
Subject: Re: [PATCH] padata: fix lockdep warning in padata serialization
Message-ID: <20220921185138.c6chlv77ugfrsukj@oracle.com>
References: <20220919151248.smfo7nq6yoqzy2vo@oracle.com>
 <20220920003908.391835-1-eadavis@sina.com>
 <20220920014711.bvreurf4ex44w6oj@oracle.com>
 <20220920055443.GI2950045@gauss3.secunet.de>
 <20220920141057.cy54d5ukflrgay3a@oracle.com>
 <20220921073616.GZ2950045@gauss3.secunet.de>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220921073616.GZ2950045@gauss3.secunet.de>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?s0eggLRn7bau+Bh39h7yAUC5NGJbfDXf56EBbnzZl4ADwbOV7xQcs37YXvxX?=
 =?us-ascii?Q?b5hAntuyCyZ1Nb67gxrcC4wepx5yb0dfOVr5Mjcz7k/bqiglUF/qu4FfOOwE?=
 =?us-ascii?Q?67Myw0eOED8xJDS709QibHHsAkd0ojCMcYrIeWY3o86eSacyD/peoow5FsHO?=
 =?us-ascii?Q?J3D6rk76eAb2kCkJIk1wtAwYZsajsoAOhQd0IHQ7n7IuhjLQ4MyLGBPwGVKO?=
 =?us-ascii?Q?o87YABJqJpj/qKTJAE4sFIitF/oewtu275zCy6LqKQtAXpLUHNHQ7GE505as?=
 =?us-ascii?Q?sahqWKYKTl3V3ZJisAzJHULt6hVHUeHUcn/3pHcUN92FmBI1xhe0rowhUHPH?=
 =?us-ascii?Q?O3+O4vQEsKn1MH0qC6KTQTOvZUFuxMyotnR5e2ZCB7EfnktMDJoI9IQS/ZSi?=
 =?us-ascii?Q?MbLbtGnTFdYwFMifh4lkyaPVBdXHi9nRfnyf9uqRioRnsyAfDeKF3s3bDFix?=
 =?us-ascii?Q?LkG8B6cKD+c6Srujh51wk0pNv34gzunK46vPT9M3/09eswGHPPYcyFo3fFpC?=
 =?us-ascii?Q?4JdwFJKENkE9MnMPvKjOwlW5Ie203WgX68yKeN/zpR+Wl8VPpGn9SODkzQVB?=
 =?us-ascii?Q?OwsUtF9O5EqrH4af+jXm+ZRuiy5Fr44MfaZOdaFpoIvXTcuR/Iiqa8zi8WSe?=
 =?us-ascii?Q?pM7deDz25KXvILx0M3nDSynbXE1s+oVCJ94CpXu6W0trpdRQxSJvbYYpNuJv?=
 =?us-ascii?Q?gTwDS8RncvnUFlM6bV4fSR0QPTABKitVSUwIpzqkU8H7bwwh8mV/+Gm7TI8n?=
 =?us-ascii?Q?Nd3oTtmC9cRd6gWHtnF0C5s7puMHbdz0bqzywN6k01rEc4wl281i0cWMN/sg?=
 =?us-ascii?Q?VaB+AkjcfRsPQGULMAhfolYpT+GFwRRPiQPtVmLkCFGL/cYsQj0Bw0aLWyGO?=
 =?us-ascii?Q?wPn+lW+rD4myfIqUVSoKonZuaNZdhm0OgixZy4qoPM5T40otaghv6jVDO+hi?=
 =?us-ascii?Q?pDbHXN1yvCJ0Zl+/zHP7Oq9V1BeeVEm+4UzknbfTV+0WjVaygAhfYFbkco8D?=
 =?us-ascii?Q?uF4Zbw97UI6oRZUz3i/WSyDMEej2jYdI5xvoXYaFGETPeLrp9uQseura3nNC?=
 =?us-ascii?Q?Pdu5ifySt2npMDpOLPjaVn0Bqkwz2nXevtoQICl2OaXjrpfQ00ZRO3a5vmgE?=
 =?us-ascii?Q?/9jpNFsGpYHzNfW98fbq7epZYwP1oI6mh4VbNglhEgTX6zx7dLHQPJzDqGJT?=
 =?us-ascii?Q?vJqZuAxdnXE4JsxMOTjGBxHiZCI5d5TaGHPWZ6Fhpg2ioqXOiObASPAHdjPk?=
 =?us-ascii?Q?KbjOsuXjmTm/hjj8/oY5A6M44V4IosP80fpyJCKamJ6sFUd9Yt4TryfOhuhh?=
 =?us-ascii?Q?zicCXsLPKLpxGBFWrn3tJ9pxUiRpAZhZrpxIqwAtqN+cSyNExgk8Kc+Fc6So?=
 =?us-ascii?Q?Dqq0LzosBJlU40j8sVFxCawdZQzFMhr9NpB66m0OyfWNhzY5HdlLQVSzWcgd?=
 =?us-ascii?Q?VJ7l9tKgXXdwzdIbSr+hTIiri91HlCoPHRQrC9J4V0aYE4ASEWHv92wKm+z2?=
 =?us-ascii?Q?BdOG5Zj2liu8menAhnOitHeL6xL2VSzkDFqwpt1vaHKnlG//CVcplL+RRnsS?=
 =?us-ascii?Q?uMPB6mrS8Phd+e+JX5y60oM7zIH8ua+VAnWoK4g8S0eOe2x/AFtBBlgfF/l0?=
 =?us-ascii?Q?Sw=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bb85bbba-38cd-4aa0-0afa-08da9c0252d8
X-MS-Exchange-CrossTenant-AuthSource: SA1PR10MB5711.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Sep 2022 18:51:42.4478
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: giMgrqAeVWyKEYwlRc0XF6efMB4cQLRev1t2mRvFngDENoo3BFyNlDrRDG6bPyvRTNJQtfmYdEml1oJlp3oEE+S2qUXdSexhnIPsa2RS1Go=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4720
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-21_09,2022-09-20_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 phishscore=0 mlxlogscore=999
 bulkscore=0 suspectscore=0 adultscore=0 spamscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000
 definitions=main-2209210126
X-Proofpoint-GUID: nOe2zmgZHZrVzM0qXWNEGewvrxrIUeml
X-Proofpoint-ORIG-GUID: nOe2zmgZHZrVzM0qXWNEGewvrxrIUeml
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On Wed, Sep 21, 2022 at 09:36:16AM +0200, Steffen Klassert wrote:
> On Tue, Sep 20, 2022 at 10:10:57AM -0400, Daniel Jordan wrote:
> > Yeah, padata_do_serial can be called with BHs off, like in the tipc
> > stack, but there are also cases where BHs can be on, like lockdep said
> > here:
> 
> padata_do_serial was designed to run with BHs off, it is a bug if it
> runs with BHs on. But I don't see a case where this can happen. The
> only user of padata_do_serial is pcrypt in its serialization callbacks
> (pcrypt_aead_enc, pcrypt_aead_dec) and the async crypto callback
> pcrypt_aead_done. pcrypt_aead_enc and pcrypt_aead_dec are issued via
> the padata_serial_worker with the padata->serial call. BHs are
> off here. The crypto callback also runs with BHs off.
> 
> What do I miss here?

Ugh.. this newer, buggy part of padata_do_parallel:

  /* Maximum works limit exceeded, run in the current task. */
  padata->parallel(padata);

This skips the usual path in padata_parallel_worker, which disables BHs.
They should be left off in the above case too.

What about this?

---8<---

Subject: [PATCH] padata: always leave BHs disabled when running ->parallel()

A deadlock can happen when an overloaded system runs ->parallel() in the
context of the current task:

    padata_do_parallel
      ->parallel()
        pcrypt_aead_enc/dec
          padata_do_serial
            spin_lock(&reorder->lock) // BHs still enabled
              <interrupt>
                ...
                  __do_softirq
                    ...
                      padata_do_serial
                        spin_lock(&reorder->lock)

It's a bug for BHs to be on in _do_serial as Steffen points out, so
ensure they're off in the "current task" case like they are in
padata_parallel_worker to avoid this situation.

Reported-by: syzbot+bc05445bc14148d51915@syzkaller.appspotmail.com
Fixes: 4611ce224688 ("padata: allocate work structures for parallel jobs from a pool")
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
---
 kernel/padata.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/padata.c b/kernel/padata.c
index e5819bb8bd1d..97f51e0c1776 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -207,14 +207,16 @@ int padata_do_parallel(struct padata_shell *ps,
 	pw = padata_work_alloc();
 	spin_unlock(&padata_works_lock);
 
+	if (!pw) {
+		/* Maximum works limit exceeded, run in the current task. */
+		padata->parallel(padata);
+	}
+
 	rcu_read_unlock_bh();
 
 	if (pw) {
 		padata_work_init(pw, padata_parallel_worker, padata, 0);
 		queue_work(pinst->parallel_wq, &pw->pw_work);
-	} else {
-		/* Maximum works limit exceeded, run in the current task. */
-		padata->parallel(padata);
 	}
 
 	return 0;
-- 
2.37.2