Hi Zdenek, I really appreciate your efforts to come up with a better patch that what I have posted to the list. To be honest, I don't really like my patch but it's the best I could come up with without a testsuite (or setting up a test environment myself for which I don't have time now anyway).
Read on, there's more at the bottom :-) Zdenek Dohnal writes: > On 03/05/2017 10:40 AM, Olaf Meeuwissen wrote: >> Hi Zdenek, >> >> Zdenek Dohnal writes: >> >>> I tried to enhanced Olaf's patch and I posted it here: >>> >>> https://paste.fedoraproject.org/paste/qssgq4s0Vtqw6R5wkDWoEV5M1UNdIGYhyRLivL9gydE= >>> >>> Were my thoughts right and will it solve this issue? >> Thinking you just "backported" the patch (it applies with a fuzz but >> otherwise cleanly against 1.0.25) and removed the comments, I almost >> overlook your code change! >> >> I think it's my FIXME that misled you but you should *not* substract >> req.value_size. [...] >> >> Hope this clarifies a bit, > > Thank you so much for explanation, Olaf. I did not notice that fact > about req.value_size. So what about fetching string length from > sanei_w_array function by parameters sent by reference? Is it acceptable > to change number and type of parameters of functions? I created patch > proposal: > > https://paste.fedoraproject.org/paste/KVJpdlIAMcxiovnYF4dhbV5M1UNdIGYhyRLivL9gydE= > > It is probably not final version, but I hope I demonstrated my idea. It > was compiled without error. Whether it compiles or not is not the important part ;-) It's gotta work! Are you able to test? If not, can you find someone who can? Maybe Kritphong? If not, the whole thing becomes a rather pointless endeavour. Having poured over the code for the better part of a weekend, I'd say the transmission of strings should not be treated as the transmission of an array (of characters). It looks to me like the sanei_w_array() code can be used fine when transferring the constraint member of a SANE_Option_Descriptor but I am not convinced it is the right thing to use when *getting* an option's SANE_String value. When getting an option's SANE_String value, the code *should* allocate a buffer big enough to hold the *largest* possible string even if the net backend is sending a (much) smaller string. The size of the largest possible string is given by the SANE_Option_Descriptor's size member for options that have an option value type of SANE_TYPE_STRING. # Please refer to the API spec for the details. Based on a quick look at your patch, you may be heading in the right direction but I'd really like to see this confirmed by: - tests indicating that saned works (as in you can get/set options with string values and scan without trouble) - packet captures that show no uninitialized bits of memory go fly over the wire (we know that the third party hpaio backend will trigger these from Kritphong's bug report so that would be a good backend to test with). - (optionally but very much recommended) an indication that there are no memory issues in saned (think valgrind logs) That's quite a bit of work and testing for which I unfortunately do not have the time right now. If you do, then, by all means, go ahead and whip up a real fix to replace my somewhat iffy patch. Hope this helps, -- Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27 GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13 F43E B8A4 A88A F84A 2DD9 Support Free Software https://my.fsf.org/donate Join the Free Software Foundation https://my.fsf.org/join
