[alsa-devel] question about sound/pci/ctxfi/ctpcm.c
The file sound/pci/ctxfi/ctpcm.c contains the functions ct_pcm_playback_open and ct_pcm_capture_open that contain the following pattern of code:
runtime->private_data = apcm; ... if (err < 0) { kfree(apcm); return err; }
I wonder if this leaves a dangling pointer to apcm in runtime? The function ct_atc_pcm_free_substream on the other hand does set the private_data field to NULL after freeing apcm. But perhaps there is something in the calling context of open that ensures that if the open function fails, the private_data field of runtime will never be used?
thanks, julia
Julia Lawall wrote:
The file sound/pci/ctxfi/ctpcm.c contains the functions ct_pcm_playback_open and ct_pcm_capture_open that contain the following pattern of code:
runtime->private_data = apcm;... if (err < 0) { kfree(apcm); return err; }
I wonder if this leaves a dangling pointer to apcm in runtime?
The runtime structure contains data that is valid only while the substream is open; it is allocated by the ALSA framework before calling the open callback, and deallocated after calling the close callback (or if the open callback fails).
The function ct_atc_pcm_free_substream on the other hand does set the private_data field to NULL after freeing apcm.
This is superfluous.
But perhaps there is something in the calling context of open that ensures that if the open function fails, the private_data field of runtime will never be used?
If the open callback fails, the close callback will not be called. However, the runtime->private_free callback, if set, will be called.
So there is indeed a dangling pointer.
Regards, Clemens
On Tue, 9 Nov 2010, Clemens Ladisch wrote:
Julia Lawall wrote:
The file sound/pci/ctxfi/ctpcm.c contains the functions ct_pcm_playback_open and ct_pcm_capture_open that contain the following pattern of code:
runtime->private_data = apcm;... if (err < 0) { kfree(apcm); return err; }
I wonder if this leaves a dangling pointer to apcm in runtime?
The runtime structure contains data that is valid only while the substream is open; it is allocated by the ALSA framework before calling the open callback, and deallocated after calling the close callback (or if the open callback fails).
The function ct_atc_pcm_free_substream on the other hand does set the private_data field to NULL after freeing apcm.
This is superfluous.
But perhaps there is something in the calling context of open that ensures that if the open function fails, the private_data field of runtime will never be used?
If the open callback fails, the close callback will not be called. However, the runtime->private_free callback, if set, will be called.
So there is indeed a dangling pointer.
Thanks for the explanation. One option would be to move the initializations downwards, since the intervening calls that use runtime don't use those fields. The other would be to set provate_data to NULL after doing the free. Any preference?
thanks, julia
Julia Lawall wrote:
[...] One option would be to move the initializations downwards, since the intervening calls that use runtime don't use those fields. The other would be to set provate_data to NULL after doing the free. Any preference?
I notice that ct_atc_pcm_free_substream does not expect private_data to be NULL, so only the first option would work. Furthermore, apcm->timer must not be NULL, so the initialization must be moved to the end of the function.
Regards, Clemens
On Tue, 9 Nov 2010, Clemens Ladisch wrote:
Julia Lawall wrote:
[...] One option would be to move the initializations downwards, since the intervening calls that use runtime don't use those fields. The other would be to set provate_data to NULL after doing the free. Any preference?
I notice that ct_atc_pcm_free_substream does not expect private_data to be NULL, so only the first option would work. Furthermore, apcm->timer must not be NULL, so the initialization must be moved to the end of the function.
Thanks. I will send a patch later today.
julia
participants (2)
-
Clemens Ladisch -
Julia Lawall