Skip to content

Conversation

@adrianM27
Copy link
Contributor

No description provided.

compress.c Outdated
ret = _write(fd, &dst_len, sizeof(dst_len));
if (ret != sizeof(dst_len))
{
fprintf(stderr, "%s() write dst_len failed: %d\n", __func__, ret);
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use err() when you write to stderr in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

memcr.c Outdated

#ifdef CHECKSUM_MD5
if (checksum && ret > 0)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

this part is already in pull request 105, please rebase once it is merged

return -1;
}

dumped_vm_size -= vmr->len;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what case dumped_vm_size helps with? is checking control sum for each vmr insufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found this useful to limit the amount of memory returned back to PID. In case of corruption/hack od dump file, memcr can try to return to PID more than platform can stand causing OOM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The control sum is verified only after the memory is returned to the PID, which means the system may already be affected by excessive data written to the PID's virtual memory

memcr.c Outdated

ret = dump_write(fd, vmr, sizeof(struct vm_region));
if (ret != sizeof(struct vm_region))
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

please put the opening brace at the end of "if" line - as commented in previous commit

compress.c Outdated

ret = _write(fd, dst, dst_len);
if (ret != dst_len)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

please put the opening brace at the end of "if" line - as commented in previous commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@adrianM27 adrianM27 force-pushed the improve_restore_error_handling branch 3 times, most recently from acfcd6a to 6018d8f Compare July 17, 2025 07:59
@meecash
Copy link
Contributor

meecash commented Jul 17, 2025

It would be good to add an error print as well on read ail in libencrypt.c:

lib__read()
if (!cipher)
return read(fd, buf, count);

@adrianM27 adrianM27 force-pushed the improve_restore_error_handling branch from 6018d8f to be7335c Compare July 21, 2025 09:19
@adrianM27
Copy link
Contributor Author

It would be good to add an error print as well on read ail in libencrypt.c:

lib__read()
if (!cipher)
return read(fd, buf, count);

Err log added here as well

libencrypt.c Outdated

if (!cipher)
if (!cipher) {
err("cipher not initialized\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not an error, !cipher here means that lib__read() executes but encryption was not enabled with -e opt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, removed this change as not needed

libencrypt.c Outdated

if (!cipher)
if (!cipher) {
err("cipher not initialized\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here

  * Log what failed and for which VM
  * Kill PID on error
  * Verify restored VM size
@adrianM27 adrianM27 force-pushed the improve_restore_error_handling branch from be7335c to b66c271 Compare July 22, 2025 10:58
@meecash meecash self-requested a review July 28, 2025 09:28
@meecash meecash merged commit f46af40 into LibertyGlobal:main Jul 28, 2025
5 checks passed
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.

3 participants