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

rdiff --paranoia produces (spurious?) critical errors #155

Closed
dbaarda opened this issue Aug 15, 2019 · 2 comments · Fixed by #158
Closed

rdiff --paranoia produces (spurious?) critical errors #155

dbaarda opened this issue Aug 15, 2019 · 2 comments · Fixed by #158
Assignees
Labels

Comments

@dbaarda
Copy link
Member

@dbaarda dbaarda commented Aug 15, 2019

This was reported to Debian at;

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=435901

I've confirmed it still happens with v2.0.2 in Debian testing now.

This should not be happening. It's unlikely to cause any corruption (which would require a strong-sum collision), but it might be indicating a rollsum bug that could be causing delta calculation to miss some matches, resulting in a larger delta than it should.

I will investigate.

@dbaarda
Copy link
Member Author

@dbaarda dbaarda commented Aug 15, 2019

I figured it out, and it is indeed a spurious bug in how --paranoid is implemented.

The problem is when a miss is appended here;

result = rs_appendmiss(job, 1);

It can trigger an rs_appendflush() here;

result = rs_appendflush(job);

Which can block if the output "tube" gets too full here;

return rs_tube_catchup(job);

And while its blocked and until the "tube" catches up, the scoop_pos offset into the input data is invalid. However, the paranoid checking then uses it to calculate the full block rollsum here;

RollsumUpdate(&test, job->scoop_next + job->scoop_pos,

Resulting in the paranoia checking comparing the checksum calculated at the wrong offset.

This could be fixed by just removing the paranoia checking, which I'm really tempted to do. However, I'll also look into how hard it would be to ensure the scoop_pos offset is always valid even when the tube blocks.

@dbaarda dbaarda self-assigned this Aug 15, 2019
@dbaarda
Copy link
Member Author

@dbaarda dbaarda commented Aug 16, 2019

After looking at the code in more detail I've decided that it's non-trivial to change the code to keep the scoop_pos offset valid while the tube is blocked.

The --paranoia feature is well past it's use-by date, and is now officially causing more bugs than it's uncovering. I'm just going to remove it.

@dbaarda dbaarda added the bug label Aug 17, 2019
dbaarda added a commit to dbaarda/librsync that referenced this issue Aug 17, 2019
… rdiff.

Also update NEWS.md with all changes since v2.0.2.
dbaarda added a commit that referenced this issue Aug 17, 2019
Fix #155 remove redundant and broken `--paranoia` option from rdiff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant