Page MenuHomeFreeBSD

zfs: Fix null ap->a_fsizetd NULL pointer derefernce
ClosedPublic

Authored by cy on Apr 4 2023, 8:42 PM.
Tags
None
Referenced Files
F105560289: D39418.diff
Tue, Dec 17, 4:30 PM
Unknown Object (File)
Fri, Nov 29, 4:17 AM
Unknown Object (File)
Nov 8 2024, 1:31 PM
Unknown Object (File)
Nov 8 2024, 11:44 AM
Unknown Object (File)
Oct 17 2024, 1:50 AM
Unknown Object (File)
Oct 16 2024, 12:59 PM
Unknown Object (File)
Oct 16 2024, 11:56 AM
Unknown Object (File)
Oct 16 2024, 11:55 AM
Subscribers

Details

Summary

NFS doesn't use ap->a_fsizetd resulting in a NULL pointer dereference
when reading from a ZFS backed NFS share.

Submitted by: rmacklem
Reported by: cy
Tested by: cy
Obtained from: rmacklem
Fixes: 2a58b312b62f
X-MFC with: 2a58b312b62f

This patch was written by

Test Plan

Tested locally.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

cy requested review of this revision.Apr 4 2023, 8:42 PM
This revision is now accepted and ready to land.Apr 4 2023, 8:45 PM

you can set the 'author' field instead of using submitted by

rmacklem may want to submit his own commit message

In D39418#897268, @mjg wrote:

you can set the 'author' field instead of using submitted by

rmacklem may want to submit his own commit message

I would prefer if Rick committed this himself.

Let me say, I'm certainly willing to abandon this revision and let rmacklem commit this.

You might find these little programs useful for testing.
(I realized my only ZFS setup doesn't do block whatever,
so I can only test the fallback to vn_generic_copy_file_range().)

https://people.freebsd.org/~rmacklem/testcfr.c

  • It takes two arguments that should both be file names for files that do not exist. It creates/fills in the first one and then copies chunks of it to the second, checking that the data reads back the same. It also checks a few corner cases, which should be handled in the generic code. (It takes a while to complete.)

https://people.freebsd.org/~rmacklem/copysame.c

  • This one copies the 1st megabyte to a second megabyte of the same file. You can use it as follows:
  • dd if=/dev/random of=foo bs=1M count=1
  • cat foo foo > bar
  • copysame foo
  • cmp foo bar
  • They should be identical.

Oh, and just fyi, I couldn't care less if/how my name ends
up on it. I've been at this game long enough to
learn that the only time having your name on some
chunk of code is noticed is when it's buggy or someone
does not like the way it was done (usually 10years after
you wrote it, so you cannot remember why you did it
the way you did it;-).

You might find these little programs useful for testing.
(I realized my only ZFS setup doesn't do block whatever,
so I can only test the fallback to vn_generic_copy_file_range().)

I h ave four machines with ZFS. My laptop has three external drives with two zpools on them in addition to the one on the SSD in the machine itself. Using my play pool (named t), I was able to reproduce the problem after discovering it crashing my buildworld machine during clieent installworlds.

https://people.freebsd.org/~rmacklem/testcfr.c

  • It takes two arguments that should both be file names for files that do not exist. It creates/fills in the first one and then copies chunks of it to the second, checking that the data reads back the same. It also checks a few corner cases, which should be handled in the generic code. (It takes a while to complete.)

https://people.freebsd.org/~rmacklem/copysame.c

  • This one copies the 1st megabyte to a second megabyte of the same file. You can use it as follows:
  • dd if=/dev/random of=foo bs=1M count=1
  • cat foo foo > bar
  • copysame foo
  • cmp foo bar
  • They should be identical.

cp(1) issues copy_file_range(2) for every copy I tried.

Oh, and just fyi, I couldn't care less if/how my name ends
up on it. I've been at this game long enough to
learn that the only time having your name on some
chunk of code is noticed is when it's buggy or someone
does not like the way it was done (usually 10years after
you wrote it, so you cannot remember why you did it
the way you did it;-).

Same here. I've been in this business for > 40 years. I'm more interested in getting this fixed. But I am sensitive to commit others work. It just _feels_ dishonest. The head understands fully. The heart begs to differ.

Ultimately I just want to see it fixed. One way or another.

BTW. Thanks for the two files. They've been fetched, will test tonight after $JOB.

pjd added a subscriber: pjd.

Thank you for the fix!