Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Herbert Xu <herbert@gondor.apana.org.au>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        Daniel Jordan <daniel.m.jordan@oracle.com>
Cc:     linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] crypto: pcrypt - Delay write to padata->info
Date:   Thu, 21 Oct 2021 14:30:28 -0400
Message-Id: <20211021183028.837112-1-daniel.m.jordan@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MglBUNCe/lVT5FoWhIHk7Gyzri6iaK5u3fOnxUw9TeP7wGtgVHlBRdnF1Vd0?=
 =?us-ascii?Q?NQiuU8LQmiFdRpX8mbZ6myYxCQ+Zdfeg4OjQMZ+J7mHm9Vk7rpOQW8B5GaVE?=
 =?us-ascii?Q?ZIABUr4Ea1v+qAFiotwS7Fi6JGmRI0s3Uo0k4/TvJdWsGwCptn7y6upfr4pk?=
 =?us-ascii?Q?ijrm5ZeCtXYX33cPUY+dH676RI8iRCY3K19ak7IeIJA0rubJBm/iQslis92b?=
 =?us-ascii?Q?USDVFwELdu5heqwUz4U9V3yISQuqn5zS/fMsMwkNTPj/oB8EppRCggrJhaUq?=
 =?us-ascii?Q?0mist/ni9pNBvG6E6zHRI4WWBvurq5AAO6ZdGW0hGo+8KyKNWi0ZMIrZGHGi?=
 =?us-ascii?Q?GK2cLTr2+F9FCcWTBmQGdgeBct+fYqMwjyfwRX7tuFT35QTbuBVGM0NRvsUs?=
 =?us-ascii?Q?Ps85vZU5EFvveVgiV+lAcaHtIZPNF9CnOZkisEpgB1Qo6uarPPtM+Wce0e6U?=
 =?us-ascii?Q?veYkTqG4jcNnV2YvzC+7fdIQ2hzYyjpMp0qDyNu1EN/fNS0UPvCyQUwt1vMu?=
 =?us-ascii?Q?4T3uEnjfR96IXiLKDM9RigZmQQkb1n93uXnGS8uCq2a0lFIy9cX5sg4tn+ky?=
 =?us-ascii?Q?eLX2zhlN5KzhshAeqQySaOv8u7i+RJ91CNWlJR/FMsDIzjEYwVLWcOpJmofN?=
 =?us-ascii?Q?RKkCtefwTQFLzl3TY90LJ8LvHLLqoVGXWzVj643iIMmgAPvRi3ufyC86z3At?=
 =?us-ascii?Q?RdsOqRQYW026IOUM5vAmwcN8yeLOWmNNg7fgrQlgskayR+HbJIkemADhrNpn?=
 =?us-ascii?Q?VSp/9U7cWe4/NtBYhsXd83TAe/dS/6djHDJLVxyYH3dWTJcPdTz99a31HbRw?=
 =?us-ascii?Q?EPhg1RQ6oFB+tKQUysaMwpZQ8p5TQYa3glPYF09VoBP06WDe2WmBuX5+lBAk?=
 =?us-ascii?Q?SX6FtuqxVpL/d9pkv8jJ/L9ARA5TpWI9cTrPKdmLbbrf2NwvreRTslCjBIDa?=
 =?us-ascii?Q?J4aaUm7lN0TqUSblNghmpIGc2IFvdM0morccgWwyOK5Kkga30I2tAsyknhZe?=
 =?us-ascii?Q?i/a2MfQOR1CLhbra9VTSLiYBf0bBbDXGqTm4kGQ/WHu+DM3ivKs4YLBNur8B?=
 =?us-ascii?Q?RuDReDlfso6+Ig9HXRiCJIwLeFhBILomymkC2JGfWC+myJ4604iNMXf3w1Sp?=
 =?us-ascii?Q?dvocRE2LJIdC/bb1KMdfXZrZ0pXWc33oxDz4WdkRDr0mY5PXwYVugImcBImI?=
 =?us-ascii?Q?dAdmf4eU4DSl8qXW3ZMck+Q6alguSSjKKTWX+wCFEV7cYTFY/D65G3HrRfa2?=
 =?us-ascii?Q?81P+myGokueOs0GYMADkwoSXo5N9c1WwDABcTKfxvq7Bn4n+j5uLnv6/prO9?=
 =?us-ascii?Q?Wdrcd/gLAEr4C3z7GbmXlhc0?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6f8ad336-50fb-4912-4b76-08d994c0e417
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2966.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Oct 2021 18:30:43.2758
 (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: daniel.m.jordan@oracle.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4447
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10144 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 adultscore=0 malwarescore=0
 phishscore=0 mlxlogscore=999 bulkscore=0 suspectscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001
 definitions=main-2110210092
X-Proofpoint-ORIG-GUID: EozwhAlUV72BTAb1EBmUspb1nU1oE4F_
X-Proofpoint-GUID: EozwhAlUV72BTAb1EBmUspb1nU1oE4F_
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

These three events can race when pcrypt is used multiple times in a
template ("pcrypt(pcrypt(...))"):

  1.  [taskA] The caller makes the crypto request via crypto_aead_encrypt()
  2.  [kworkerB] padata serializes the inner pcrypt request
  3.  [kworkerC] padata serializes the outer pcrypt request

3 might finish before the call to crypto_aead_encrypt() returns in 1,
resulting in two possible issues.

First, a use-after-free of the crypto request's memory when, for
example, taskA writes to the outer pcrypt request's padata->info in
pcrypt_aead_enc() after kworkerC completes the request.

Second, the outer pcrypt request overwrites the inner pcrypt request's
return code with -EINPROGRESS, making a successful request appear to
fail.  For instance, kworkerB writes the outer pcrypt request's
padata->info in pcrypt_aead_done() and then taskA overwrites it
in pcrypt_aead_enc().

Avoid both situations by delaying the write of padata->info until after
the inner crypto request's return code is checked.  This prevents the
use-after-free by not touching the crypto request's memory after the
next-inner crypto request is made, and stops padata->info from being
overwritten.

Fixes: 5068c7a883d16 ("crypto: pcrypt - Add pcrypt crypto parallelization wrapper")
Reported-by: syzbot+b187b77c8474f9648fae@syzkaller.appspotmail.com
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
---

So, pcrypt(pcrypt(...)) doesn't make all that much sense, but apparently
there's not an easy way to prevent it [1], so I'm going with the minimal
fix.

[1] https://lkml.kernel.org/20171230083744.vuclnbs677tj7pi2@gauss3.secunet.de/

 crypto/pcrypt.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index d569c7ed6c80..9d10b846ccf7 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -78,12 +78,14 @@ static void pcrypt_aead_enc(struct padata_priv *padata)
 {
 	struct pcrypt_request *preq = pcrypt_padata_request(padata);
 	struct aead_request *req = pcrypt_request_ctx(preq);
+	int ret;
 
-	padata->info = crypto_aead_encrypt(req);
+	ret = crypto_aead_encrypt(req);
 
-	if (padata->info == -EINPROGRESS)
+	if (ret == -EINPROGRESS)
 		return;
 
+	padata->info = ret;
 	padata_do_serial(padata);
 }
 
@@ -123,12 +125,14 @@ static void pcrypt_aead_dec(struct padata_priv *padata)
 {
 	struct pcrypt_request *preq = pcrypt_padata_request(padata);
 	struct aead_request *req = pcrypt_request_ctx(preq);
+	int ret;
 
-	padata->info = crypto_aead_decrypt(req);
+	ret = crypto_aead_decrypt(req);
 
-	if (padata->info == -EINPROGRESS)
+	if (ret == -EINPROGRESS)
 		return;
 
+	padata->info = ret;
 	padata_do_serial(padata);
 }
 
-- 
2.27.0