----- Original Message ----- > From: "Aurélien Aptel" <aaptel@xxxxxxxx> > To: "Ronnie Sahlberg" <lsahlber@xxxxxxxxxx>, "linux-cifs" <linux-cifs@xxxxxxxxxxxxxxx> > Cc: "Steve French" <smfrench@xxxxxxxxx> > Sent: Monday, 19 February, 2018 10:39:22 PM > Subject: Re: [PATCH 0/0] fix smb3-encryption breakage when CONFIG_DEBUG_SG=y > > Hi Ronnie, > > Ronnie Sahlberg <lsahlber@xxxxxxxxxx> writes: > > Steve, All, > > > > I think smb3-encryption has been broken for quite a while (forver?) > > when we configure the kernel with CONFIG_DEBUG_SG="y". > > > > The problem is that with these extra checks, we can no longer pass a > > pointer > > to a buffer that is allocated off the stack safely to sg_set_buf() as this > > function will BUG_ON. > > It seems to me this sg_set_buf() is quite restrictive. Is it an actual > bug to pass stack buffers or does it become one only if this debug > thing is set? IIUC it's the latter. It is the latter. > > > We have a lot of places where cifs.ko does indeed pass a pointer to a > > stack object to this function :-( and this patch will fix all the > > instances > > I could find so far. > > Switching that many stack buffers to heap doesn't sound like a good > idea, especially in hot paths :( t be > > pros: > * we can use this scatter-gather table debug thing (how?). > > cons: > * makes the code more complex: memory management in C is error-prone as you > all know (memory leaks, use-after-free, extra error code paths to add > if allocation fails, etc) > * slower: heap allocation is not free (altho we should probably measure > instead of speculate) > We need to do something since otherwise any time someone compiles their kernel with this option, smb3enc will no longer work, and the cifs module will die by BUG_ON :-( Moving everything to be kzallocated is error prone as you say and also a bit heavy handed. One alternative I see could be to just bypass the check completely and call sg_set_page() directly instead of sg_set_buf() but that does not feel right either. > I'd like to know more about this debug feature because I'm not sure why > there is this requirement. In any case thanks for finding and looking at > this. > > Cheers, > > -- > Aurélien Aptel / SUSE Labs Samba Team > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
