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

Protect against null pointer for service member #23

Open
dcaliste opened this issue Jan 15, 2025 · 1 comment
Open

Protect against null pointer for service member #23

dcaliste opened this issue Jan 15, 2025 · 1 comment

Comments

@dcaliste
Copy link
Contributor

0028-Protect-service.patch, this one looks suspicious to me. I was not able to find when these changes were added and for what reason. I guess a crash coming from a memory corruption. If one looks into qmailmessageservice.cpp:370, the service pointer is set on construction and is never changed there after. For imap for instance, this is created in imapservice.cpp:1545. Except if a user of ImapService::source() is keeping a pointer on the return value, there is no reason for the source to out-live the servcie. Indeed in servicehandler.cpp:946 and particularly in ServiceHandler::registerAccountSource(), the source pointer is saved in a map. So this could be problematic. But looking at the code, it seems that the map is flushed from the source before the service is deleted… To be investigated more. But the solution to protect the access to the service pointer as the patch is doing is useless in my opinion since the _service member can never be null by construction.

@pvuorela
Copy link
Contributor

I was not able to find when these changes were added and for what reason.

git log -S "long sync operation is ongoing and account" -> last is commit 9d5d7d6 which appears having this.

That's old and together with tons of other changes. There's a chance that it wasn't fully thought out or maybe some development time half-way done thing.

In the current form it does indeed look quite useless. Think we could just remove it, though maybe get the SSO stuff in first.

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

No branches or pull requests

2 participants