Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix off by one in zpl_lookup #5768

Merged
merged 1 commit into from
Feb 11, 2017
Merged

Fix off by one in zpl_lookup #5768

merged 1 commit into from
Feb 11, 2017

Conversation

tuxoko
Copy link
Contributor

@tuxoko tuxoko commented Feb 9, 2017

Doing the following command would return success with zfs creating an orphan
object.

touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen [email protected]

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Change has been approved by a ZFS on Linux member.

@mention-bot
Copy link

@tuxoko, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @dweeezil and @mkjorling to be potential reviewers.

@tuxoko
Copy link
Contributor Author

tuxoko commented Feb 9, 2017

BTW, I have no idea why it uses ZFS_MAX_DATASET_NAME_LEN here.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, the fix looks good. It is odd that we're not using MAXNAMELEN here. It looks like this case was accidentally swept up in the eca7b76 mass rename.

@behlendorf
Copy link
Contributor

Using ZAP_MAXNAMELEN would make far more sense.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this to ZAP_MAXNAMELEN while we're here. They're both defined to 256 as they should be so there's no functional change.

Doing the following command would return success with zfs creating an orphan
object.

	touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen <[email protected]>
@tuxoko
Copy link
Contributor Author

tuxoko commented Feb 10, 2017

Changed to ZAP_MAXNAMELEN

@behlendorf behlendorf merged commit d6df043 into openzfs:master Feb 11, 2017
wli5 pushed a commit to wli5/zfs that referenced this pull request Feb 28, 2017
Doing the following command would return success with zfs creating an orphan
object.

	touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Closes openzfs#5768
wli5 pushed a commit to wli5/zfs that referenced this pull request Feb 28, 2017
Doing the following command would return success with zfs creating an orphan
object.

	touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Closes openzfs#5768
l1k pushed a commit to l1k/zfs that referenced this pull request Apr 17, 2017
Doing the following command would return success with zfs creating an orphan
object.

	touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Closes openzfs#5768
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 18, 2017
Doing the following command would return success with zfs creating an orphan
object.

	touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Closes openzfs#5768
tonyhutter pushed a commit that referenced this pull request Jun 9, 2017
Doing the following command would return success with zfs creating an orphan
object.

	touch $(for i in $(seq 256); do printf "n"; done)

The funny thing is that this will only work once for each directory, because
after upgraded to fzap, zfs_lookup would fail properly since it has additional
length check.

Signed-off-by: Chunwei Chen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Closes #5768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants