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 memory corruption bug of cron module #271

Merged

Conversation

zhjwpku
Copy link
Contributor

@zhjwpku zhjwpku commented May 29, 2023

I found a bug of vixie cron, which bit_nset and bit_nclear are used to set and clean bits of range [start, stop], for dow and hour, they use 8 bits and 24bits, the following statement will write the next byte since the pass the length instead of the stop position, which will corrupt the memory.

bit_nset(e->hour, 0, (LAST_HOUR-FIRST_HOUR+1));
bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
bit_nclear(bits, 0, (high-low+1));

Paul vixie(the author of vixie cron) fix this with some code refactory, this patch just backport it back to pg_cron.

The original PRs:

  1. proposed fix to issue 15 vixie/cron#16
  2. fix another memory corruption bug vixie/cron#18

I found a bug of vixie cron, which bit_nset and bit_nclear are used
to set and clean bits of range [start, stop], for `dow` and `hour`,
they use 8 bits and 24bits, the following statement will write the
next byte since the pass the length instead of the stop position,
which will corrupt the memory.

bit_nset(e->hour, 0, (LAST_HOUR-FIRST_HOUR+1));
bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
bit_nclear(bits, 0, (high-low+1));

Paul vixie(the author of vixie cron) fix this with some code refactory,
this patch just backport it back to pg_cron.

The original PRs:

1. vixie/cron#16
2. vixie/cron#18

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
@zhjwpku
Copy link
Contributor Author

zhjwpku commented May 30, 2023

@marcocitus can you take some time to review this PR?

@marcocitus marcocitus merged commit 1c9fa93 into citusdata:main Jun 16, 2023
@marcocitus
Copy link
Member

Thanks!

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.

2 participants