jpc_dec: fix tile memory leak after decoder failure #159
Conversation
The function jpc_dec_tilefini() is only called for tiles which have completed successfully. If there is one decoder error, all unfinished tiles leak lots of memory. This change adds jpc_dec_tilefini() calls to jpc_dec_destroy() for each tile which is not marked "done" (state!=JPC_TILE_DONE). This however crashes when the tile initialization loop in jpc_dec_process_siz() fails, because the rest of the array has not yet been initialized; to work around this, I added a loop which initializes all states with JPC_TILE_DONE before doing the real initialization.
|
@mdadams I think this is about CVE-2017-13748 |
|
Since this repository is abandoned and @mdadams has not replied for several years to my code submissions and emails, I'll be moving my efforts to fix Jasper to https://github.com/jasper-maint/jasper |
@MaxKellermann and @jubalh: |
JasPer had pull requests open from 2017, with no comment on it whatsoever. There are 62 issues many of which are CVEs dating back to 2016 some of which also didn't get any feedback. In a mail that is over an year old I already suggested various ways to increase resources. Adding more people to the repo, asking some of your students to work on JasPer, creating a GitHub organization. You answered questions in that mail but omitted this topic entirely.
A dead upstream is an upstream that does not care about the project anymore. Doesn't give feedback to potential contributors nor shows any activity. Even though that upstream still has "interest", the action is what counts to the users.
In the README of our fork we explicitly state:
So we never claimed our for to be the official jasper and marked it as a fork explicitly.
My first mail about the suggestion to create a GitHub organization is over a year old. I have written numerous more after that. Always trying to keep the original JasPer alive. None such mail was answered. All this happened way before COVID-19. April 14th I again made the idea to create an organization public. By this time you didn't respond to no mail nor GitHub issue at all. Then we finally do the work. Set up the organization, push the repo. Create issues so that we can organize everything in a clean way. Do the first commits. And then you suddenly appear and feel entitled to mention your opposition to the project? COVID-19 has effect on many lives, but I wonder how it can be that you were able to ignore all mails and questions but found the time give a quick shoutout when suddenly something starts to happen. If we wouldn't have forked you would have stayed silent for months on or forever. And it's not just the silence in recent months with the question about adding more team members to jasper. The state of the old repo has plenty of bugs with no comments. Many open PRs with no feedback at all. This doesn't sound like good community work to me. In any case there are several things that can happen now. I see the following:
I would prefer the second to last option, that you join the organization. Since we organized everything quite nicely and transparently. Users will easily be able to follow the development and fixing of bugs. Things are clearly referenced. Curious what @MaxKellermann has to say about the subject. And also interesting that you commented that here and not in #208 where I think it would have been more appropriate. |
|
I agree with everything @jubalh said. |
|
@MaxKellermann and @jubalh: |
|
I see. So since we already set up an organization and have some issues and some commits. I think it would make sense if you join our organization. So all the references etc will still work. We could then either just add a note in the readme of your original repo that development moved to the new organization (where you also have access etc) or we could ask GitHub to link from there to the new repo. So that one goes to https://github.com/mdadams/jasper one gets forwarded to https://github.com/jasper-maint/jasper like if we would have transferred the repo right from the start. Since in some of our commits we referenced issues form the old repo I'm not sure what will happen to those links. We could also remove our repo, push the changes so far to yours and transfer that repo to a new organization. Not sure which is the best way.. :( It appears to me that just adding a note and link to the old readme would that directs to the repo we created and adding you as owner to jasper-maint would be best. Since then all references would still be intact. Development can continue and 3 people have access, so it's unlikely to drop dead. |
That is what @jubalh had been suggesting since last year.
Yes. That would preserve all existing links, existing bug reports and would keep the connection between all those forks. (Note: technically, https://github.com/jasper-maint/jasper/ is not a "GitHub fork"; while it is legally a fork, on the GitHub level, it is a new project, because we gave up on following https://github.com/mdadams/jasper, and that's the semantic difference to those 56 existing forks, which are not new projects.)
It is unfortunate that @mdadams waited until it was too late already. This way, he created lots of unnecessary confusion and caused us more work than necessary, costing us more time than necessary - when scarce of time is what plagues everybody. Again, I'm annoyed. To avoid further problems, my suggestion is that we continue on https://github.com/jasper-maint/jasper/ until its "issues" section is solved. By then, @mdadams will have made up his mind on how he plans to run the new organization. If we agree with his plans, we'll join it, push "jasper-maint/jasper/master" to it and delete "jasper-maint". Things for @mdadams to do/decide:
|
I see some confusion about the term here. Like @MaxKellermann mentioned these forks are kind of different. We also could have "forked" with the GitHub link your repository to jasper-maint/jasper and then worked on that. Then GitHub would have preserved the link and below our repo it would print "forked from mdadams/jasper". But we just cloned the repo. Created a new one on GitHub and pushed the repo there. So GitHub doesn't have a link to the original. So your assumption that these people forked for similar reason is wrong. It is something else entirely. None of these repos (I didn't check all) have any more commits than your repo. They were not done to keep jasper alive. I also had some time to think about all this again. And I would like to understand what exactly is your motivation behind creating jasper-software organization, and not just join our jasper-maint organization which we created roughly 2 weeks before yours after you didn't reply to the requests for over a year? Also, your original repo, if transferred will have all the issues/PRs transferred too. So I would like to know whether you will clean them up or what you plan to do with them. Because in our fork we already went through the original issues and created new issues in our repo for them. In a cleaner and more managable way. We all have limited time, @MaxKellermann and I volunteered to help with this project. But I would appreciate if it wouldn't be made so hard and more time consuming than necessary. So before now doing all the extra work again, I would like to understand why you think it needs to be done. And what you think will be improved in this way. And it would be good to know how much time etc you can/want to put in in the future. So far I don't see any benefit except extra work for us. |
|
@mdadams if you agree with the changes we did to our fork, you can easily merge them now into your official JasPer repository. That way, everybody benefits from the fixes. |
The function jpc_dec_tilefini() is only called for tiles which have
completed successfully. If there is one decoder error, all unfinished
tiles leak lots of memory.
This change adds jpc_dec_tilefini() calls to jpc_dec_destroy() for
each tile which is not marked "done" (state!=JPC_TILE_DONE). This
however crashes when the tile initialization loop in
jpc_dec_process_siz() fails, because the rest of the array has not yet
been initialized; to work around this, I added a loop which
initializes all states with JPC_TILE_DONE before doing the real
initialization.