[Drbd-dev] [PATCH] initialize tconn->peer_integrity_tfm in drbd_adm_connect()

[Drbd-dev] [PATCH] initialize tconn->peer_integrity_tfm in drbd_adm_connect()

[Thomas Zeitlhofer]

The support for data-integrity-alg seems to be broken with drbd-8.4.0.
E.g., when trying to resync, the SyncTarget hits:

   ASSERTION IS_ALIGNED(data_size, 512) FAILED in read_in_block
   error receiving RSDataReply, e: -5 l: 126996! read_in_block

This is caused by an uninitialized tconn->peer_integrity_tfm. Therefore,
the digest data is not used/removed at the receiver which leads to
additional 20 bytes that trigger the assertion above.

Initializing tconn->peer_integrity_tfm the same way as tconn->integrity_tfm
seems to fix the problem.

Signed-off-by: Thomas Zeitlhofer <thomas.zeitlhofer@nt.tuwien.ac.at>
---
 drbd/drbd_nl.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drbd/drbd_nl.c b/drbd/drbd_nl.c
index 7fd6408..8095f3b 100644
--- a/drbd/drbd_nl.c
+++ b/drbd/drbd_nl.c
@@ -1864,6 +1864,7 @@ struct crypto {
 	struct crypto_hash *csums_tfm;
 	struct crypto_hash *cram_hmac_tfm;
 	struct crypto_hash *integrity_tfm;
+	struct crypto_hash *peer_integrity_tfm;
 	void *int_dig_in;
 	void *int_dig_vv;
 };
@@ -1902,6 +1903,10 @@ alloc_crypto(struct crypto *crypto, struct net_conf *new_conf)
 		       ERR_INTEGRITY_ALG);
 	if (rv != NO_ERROR)
 		return rv;
+	rv = alloc_hash(&crypto->peer_integrity_tfm, new_conf->integrity_alg,
+		       ERR_INTEGRITY_ALG);
+	if (rv != NO_ERROR)
+		return rv;
 	if (new_conf->cram_hmac_alg[0] != 0) {
 		snprintf(hmac_name, CRYPTO_MAX_ALG_NAME, "hmac(%s)",
 			 new_conf->cram_hmac_alg);
@@ -1928,6 +1933,7 @@ static void free_crypto(struct crypto *crypto)
 	kfree(crypto->int_dig_vv);
 	crypto_free_hash(crypto->cram_hmac_tfm);
 	crypto_free_hash(crypto->integrity_tfm);
+	crypto_free_hash(crypto->peer_integrity_tfm);
 	crypto_free_hash(crypto->csums_tfm);
 	crypto_free_hash(crypto->verify_tfm);
 }
@@ -2152,6 +2158,7 @@ int drbd_adm_connect(struct sk_buff *skb, struct genl_info *info)
 	tconn->int_dig_vv = crypto.int_dig_vv;
 	tconn->cram_hmac_tfm = crypto.cram_hmac_tfm;
 	tconn->integrity_tfm = crypto.integrity_tfm;
+	tconn->peer_integrity_tfm = crypto.peer_integrity_tfm;
 	tconn->csums_tfm = crypto.csums_tfm;
 	tconn->verify_tfm = crypto.verify_tfm;
 
-- 
1.7.2.5

Re: [Drbd-dev] [PATCH] initialize tconn->peer_integrity_tfm in drbd_adm_connect()

[Andreas Gruenbacher]

Thomas,

On Monday 25 July 2011 01:38:48 Thomas Zeitlhofer wrote:
> The support for data-integrity-alg seems to be broken with drbd-8.4.0.
> E.g., when trying to resync, the SyncTarget hits:
> 
>    ASSERTION IS_ALIGNED(data_size, 512) FAILED in read_in_block
>    error receiving RSDataReply, e: -5 l: 126996! read_in_block
> 
> This is caused by an uninitialized tconn->peer_integrity_tfm. Therefore,
> the digest data is not used/removed at the receiver which leads to
> additional 20 bytes that trigger the assertion above.

thanks for tracking this down.  Your analysis and the fix are correct.  As it 
happens, we also ran into this bug here meanwhile.  Here is what we did to 
address it:

  http://git.drbd.org/?p=drbd-8.4.git;a=commit;h=4811eceb393c274f94bfd8926e714d6271cf1f57

The two fixes are slightly different, but both should do the job.  Sorry for 
not letting you know of our fix in time.

The master branch in http://git.drbd.org/?p=drbd-8.4.git contains a few 
additional minor fixes.

Thanks,
Andreas

Re: [Drbd-dev] [PATCH] initialize tconn->peer_integrity_tfm in drbd_adm_connect()

[Thomas Zeitlhofer]

Hello Andreas,

On Mon, Jul 25, 2011 at 11:39:45AM +0200, Andreas Gruenbacher wrote:
> Thomas,
> 
> On Monday 25 July 2011 01:38:48 Thomas Zeitlhofer wrote:
> > The support for data-integrity-alg seems to be broken with drbd-8.4.0.
> > E.g., when trying to resync, the SyncTarget hits:
> > 
> >    ASSERTION IS_ALIGNED(data_size, 512) FAILED in read_in_block
> >    error receiving RSDataReply, e: -5 l: 126996! read_in_block
> > 
> > This is caused by an uninitialized tconn->peer_integrity_tfm. Therefore,
> > the digest data is not used/removed at the receiver which leads to
> > additional 20 bytes that trigger the assertion above.
> 
> thanks for tracking this down.  Your analysis and the fix are correct.  As it 
> happens, we also ran into this bug here meanwhile.  

I stumbled upon this bug and was already wondering that it has not been
discussed so far.

> Here is what we did to address it:
> 
>   http://git.drbd.org/?p=drbd-8.4.git;a=commit;h=4811eceb393c274f94bfd8926e714d6271cf1f57
> 
> The two fixes are slightly different, but both should do the job.
> Sorry for not letting you know of our fix in time.
>
> The master branch in http://git.drbd.org/?p=drbd-8.4.git contains a few 
> additional minor fixes.

Thanks for the information - the public git repository did not show any
commits since the 8.4.0 release, so I just had a look at this issue.

Best Regards

Thomas

Re: [Drbd-dev] [PATCH] initialize tconn->peer_integrity_tfm in drbd_adm_connect()

[Andreas Gruenbacher]

On Monday 25 July 2011 14:36:57 Thomas Zeitlhofer wrote:
> On Mon, Jul 25, 2011 at 11:39:45AM +0200, Andreas Gruenbacher wrote:
> > The master branch in http://git.drbd.org/?p=drbd-8.4.git contains a few 
> > additional minor fixes.
> 
> Thanks for the information - the public git repository did not show any
> commits since the 8.4.0 release, so I just had a look at this issue.

Yes ... you should see all commits up to "drbd: remove disfunctional 
NLM_F_REPLACE usage" from Lars now.

Thanks,
Andreas

Re: [Drbd-dev] [PATCH] initialize tconn->peer_integrity_tfm in drbd_adm_connect()

[Thomas Zeitlhofer]

On Mon, Jul 25, 2011 at 02:47:22PM +0200, Andreas Gruenbacher wrote:
> On Monday 25 July 2011 14:36:57 Thomas Zeitlhofer wrote:
> > On Mon, Jul 25, 2011 at 11:39:45AM +0200, Andreas Gruenbacher wrote:
> > > The master branch in http://git.drbd.org/?p=drbd-8.4.git contains a few 
> > > additional minor fixes.
> > 
> > Thanks for the information - the public git repository did not show any
> > commits since the 8.4.0 release, so I just had a look at this issue.
> 
> Yes ... you should see all commits up to "drbd: remove disfunctional 
> NLM_F_REPLACE usage" from Lars now.

Yes, these commits do show up now.

Thanks,

Thomas