From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754722AbcBPJPz (ORCPT ); Tue, 16 Feb 2016 04:15:55 -0500 Received: from zimbra13.linbit.com ([212.69.166.240]:54200 "EHLO zimbra13.linbit.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754296AbcBPJPn (ORCPT ); Tue, 16 Feb 2016 04:15:43 -0500 X-Greylist: delayed 405 seconds by postgrey-1.27 at vger.kernel.org; Tue, 16 Feb 2016 04:15:42 EST Date: Tue, 16 Feb 2016 10:08:51 +0100 From: Lars Ellenberg To: Insu Yun Cc: philipp.reisner@linbit.com, drbd-dev@lists.linbit.com, linux-kernel@vger.kernel.org, taesoo@gatech.edu, yeongjin.jang@gatech.edu, insu@gatech.edu, changwoo@gatech.edu Subject: Re: [PATCH] drbd: correctly handling failed crypto_alloc_hash Message-ID: <20160216090851.GS29699@soda.linbit> Mail-Followup-To: Insu Yun , philipp.reisner@linbit.com, drbd-dev@lists.linbit.com, linux-kernel@vger.kernel.org, taesoo@gatech.edu, yeongjin.jang@gatech.edu, insu@gatech.edu, changwoo@gatech.edu References: <1455589585-7275-1-git-send-email-wuninsu@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1455589585-7275-1-git-send-email-wuninsu@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 15, 2016 at 09:26:25PM -0500, Insu Yun wrote: > crypto_alloc_hash returns an error code, not NULL. You are correct, of course. Was broken since its introduction five years ago. Strange though, we have a helper function further down in that file, and other, even much older, call sites as well, which are doing the IS_ERR() correctly. Apparently no-one ever requested a non-supported alg. > Signed-off-by: Insu Yun > --- > drivers/block/drbd/drbd_receiver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c > index 1957fe8..9063462 100644 > --- a/drivers/block/drbd/drbd_receiver.c > +++ b/drivers/block/drbd/drbd_receiver.c > @@ -3403,7 +3403,7 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in > */ > > peer_integrity_tfm = crypto_alloc_hash(integrity_alg, 0, CRYPTO_ALG_ASYNC); > - if (!peer_integrity_tfm) { > + if (IS_ERR(peer_integrity_tfm)) { > drbd_err(connection, "peer data-integrity-alg %s not supported\n", > integrity_alg); > goto disconnect; Your patch is incomplete, though: the first action in the "disconnect" cleanup path is crypto_free_hash(peer_integrity_tfm); so we better make sure it is not trying to free an error pointer: diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c index c097909..6054c53 100644 --- a/drivers/block/drbd/drbd_receiver.c +++ b/drivers/block/drbd/drbd_receiver.c @@ -3376,7 +3376,8 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in */ peer_integrity_tfm = crypto_alloc_hash(integrity_alg, 0, CRYPTO_ALG_ASYNC); - if (!peer_integrity_tfm) { + if (IS_ERR(peer_integrity_tfm)) { + peer_integrity_tfm = NULL; drbd_err(connection, "peer data-integrity-alg %s not supported\n", integrity_alg); goto disconnect; Thanks, Lars