On 7/24/19 2:25 PM, Mark Balançian wrote: > Hi Ezequiel, > > (sorry didn't include linux-media in first email) > I'm not sure yet if I have my supervisor's permission to declare our > tool as open source, but I'll tell you the possible code paths that I > think may be leading our tool to think what it's thinking. > > First off, it detects a write access to desc->virt without locks inside > tw686x_memcpy_data_free, after it is called in the calling chain > tw686x_probe -> allocate an interrupt line -> tw686x_video_init -> > tw686x_set_format -> tw686x_memcpy_dma_free. Until the video device has been registered there is no need for locking since there is no possibility of concurrent access. Further, > spin_lock_init(&dev->lock) (line 319 of tw686x-core.c) isn't > correspondingly closed in the function. Is this intended? Huh? spin_lock_init doesn't take a lock. It just initializes internal data. > > Second, there is a possibility according to how I have traced a call > chain that tw686x_init is going to the error: label since tw686x_init? I assume you mean tw686x_video_init? > tw686x_memcpy_dma_free is getting called inside another possible calling > chain, going tw686x_init -> tw686x_video_free (error: label) -> > dma_ops->free (i.e. tw686x_memcpy_dma_free). I would assume this would > not be intended either. What tw686x_video_free() does really should be done in the release function of the video_device: vdev->release is currently set to video_device_release, but that should be a custom function that calls dev->dma_ops->free. > In addition, our tool detects a read access without locks to desc->virt > inside tw686x_audio_irq (line 72 of tw686x-audio.c). Not sure what you > make of that, but I'd be keen on hearing about that as well from you. No locks should be needed for that. It is still not clear what your actual problem is: do you have crashes sometimes? Is there really something wrong? Regards, Hans
