linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve Magnani <steve.magnani@digidescorp.com>
To: Jan Kara <jack@suse.cz>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org
Subject: Possible UDF locking error?
Date: Sat, 23 Mar 2019 15:14:05 -0500	[thread overview]
Message-ID: <224e1613-b080-bd64-ef88-badcb755a233@digidescorp.com> (raw)

Hi,

I have been hunting a UDF bug that occasionally results in generation
of an Allocation Extent Descriptor with an incorrect tagLocation. So
far I haven't been able to see a path through the code that could
cause that. But, I noticed some inconsistency in locking during
AED generation and wonder if it could result in random corruption.

The function udf_update_inode() has this general pattern:

   bh = udf_tgetblk(...);   // calls sb_getblk()
   lock_buffer(bh);
   memset(bh->b_data, 0, inode->i_sb->s_blocksize);
   
   // <snip>other code to populate FE/EFE data in the block</snip>
   
   set_buffer_uptodate(bh);
   unlock_buffer(bh);
   mark_buffer_dirty(bh);

This I can understand - the lock is held for as long as the buffer
contents are being assembled.

In contrast, udf_setup_indirect_aext(), which constructs an AED,
has this sequence:

   bh = udf_tgetblk(...);   // calls sb_getblk()
   lock_buffer(bh);
   memset(bh->b_data, 0, inode->i_sb->s_blocksize);

   set_buffer_uptodate(bh);
   unlock_buffer(bh);
   mark_buffer_dirty_inode(bh);

   // <snip>other code to populate AED data in the block</snip>

In this case the population of the block occurs without
the protection of the lock.

Because the block has been marked dirty, does this mean that
writeback could occur at any point during population?

There is one path through udf_setup_indirect_aext() where
mark_buffer_dirty_inode() gets called again after population is
complete, which I suppose could heal a partial writeout, but there is
also another path in which the buffer does not get marked dirty again.

Regards,
  ------------------------------------------------------------------------
  Steven J. Magnani               "I claim this network for MARS!
  www.digidescorp.com              Earthling, return my space modulator!"

  #include <standard.disclaimer>


             reply	other threads:[~2019-03-23 20:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-23 20:14 Steve Magnani [this message]
2019-03-25 16:42 ` Possible UDF locking error? Jan Kara
2019-03-25 18:23   ` Steve Magnani
2019-03-30 19:49   ` Steve Magnani
2019-04-03  8:07     ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=224e1613-b080-bd64-ef88-badcb755a233@digidescorp.com \
    --to=steve.magnani@digidescorp.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).