Hi, Lucas! >>>>> On Tue, 29 Jan 2019 08:50:10 -0800, Lucas De Marchi wrote: > On Tue, Jan 29, 2019 at 11:50:05AM +0200, Yauheni Kaliuta wrote: >> Hi, Lucas! >> >>>>>>> On Mon, 28 Jan 2019 10:05:24 -0800, Lucas De Marchi wrote: >> > On Sat, Jan 26, 2019 at 3:01 AM Yauheni Kaliuta >> > <yauheni.kaliuta@xxxxxxxxxx> wrote: >> >> >> >> [...] >> >> >> + >> >> >> + pvt->cms = cms; >> >> >> + pvt->key_id = key_id_str; >> >> >> + pvt->sno = sno_bn; >> >> >> + sig_info->private = pvt; >> >> >> >> > why do you keep pvt around if the only thing you will do with >> >> > it later is to free it? >> >> > AFAICS the only thing that needs to remain around is the str >> >> > so we can free it after the user used it (because normal >> >> > signature is backed in memory by the mem object, while these >> >> > are openssl structs) >> >> >> >> I should keep them until kmod_module_get_info() makes the copies. >> >> >> >> cms is openssl struct >> >> sno_bn is allocated by openssl and must be freed later >> >> key_id_str is allocated here since the size in unknown in advance >> >> and must be freed later. >> >> >> >> Or what did I miss? >> >> > we could just duplicate the information that we want stored and keep >> > the openssl context contained >> > to just this function. I thought the only one would be key_str_id, but >> > missed that sig and signer >> > also need to have their backing object around. >> >> If I duplicate it here then without cleanup I'll have memory >> leak, no? > yes, my idea was to just leave it simpler and add a > if (pkcs7) > free(key_id) Ah, got it, thanks! Well, if it's not a show stopper for you, I would rather keep it as in the original proposal since it does not inject implementation details to the upper level. > Lucas De Marchi >> >> In the old code they were pointers inside the module image and >> freed with the image itself. >> >> -- >> WBR, >> Yauheni Kaliuta -- WBR, Yauheni Kaliuta
