Re: [PATCH] udf: propagate error from udf_fiiter_add_entry

[Jan Kara]

Hi!

On Tue 21-04-26 16:23:24, Tristan wrote:
> Hi Jan,
> 
> You're interacting with a human (with good will and intent), not an LLM.
> 
> I do use AI tooling in my workflow (as I disclosed to Johannes), but the
> analysis above and the proposed approach are my own.

I'm sorry. The tone of your reply triggered my "I'm talking to LLM"
detector and I didn't feel like educating LLM about where it is wrong.
Below is an explanation what is problematic with your suggestion.

> Le mar. 21 avr. 2026 à 14:36, Jan Kara <jack@xxxxxxx> a écrit :
> > On Tue 21-04-26 10:43:21, Tristan Madani wrote:
> > > On Mon, 20 Apr 2026, Jan Kara wrote:
> > > > Thanks for the patch. Did you analyze how it happened that the
> > directory
> > > > iteration fixing up tags failed? Because if it fails like this, we have
> > > > practically wiped the directory which is not great. So if we are
> > missing
> > > > some directory consistency check before we start converting it, it
> > would be
> > > > good to add it instead of just failing like this later.
> > >
> > > Good point. The syzbot reproducer uses a crafted filesystem image where
> > > the inline directory data passes inode loading but contains entries that
> > > fail udf_verify_fi() during iteration (e.g., bad tagIdent or CRC length
> > > mismatch).
> > >
> > > The problem is that by the time we iterate in udf_expand_dir_adinicb()
> > > to fix up tagLocation, we've already changed the allocation type and
> > > moved the data to a new block. If the iteration then fails, the
> > > directory is in an inconsistent state.
> > >
> > > The right fix would be to validate the inline directory entries
> > > BEFORE starting the conversion -- iterate once to verify all entries
> > > pass udf_verify_fi(), then proceed with the block allocation and tag
> > > fixup only if validation succeeds. That way we never start a
> > > destructive conversion on data we can't fully iterate.
> > >
> > > I can send a v2 with this approach: a pre-conversion validation pass
> > > using udf_fiiter_init/advance (which calls udf_verify_fi internally),
> > > and only proceed if the full iteration succeeds. The error propagation
> > > from v1 would remain as a safety net in case anything goes wrong
> > > despite the pre-check.
> > >
> > > Would that be the approach you have in mind?

Not quite. udf_expand_dir_adinicb() is called only from
udf_fiiter_add_entry(). Before we get to calling udf_expand_dir_adinicb()
we must have already iterated the whole directory so at that point all
directory entries have passed udf_verify_fi(). The directory is locked all
the time so it is unclear how a few lines later udf_verify_fi() can fail in
iteration in udf_expand_dir_adinicb(). So this we need to understand first
before deciding which check should be added where...

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR