Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ALSA: firewire-lib: fix buffer-over-run when detecting packet discontinuity

When detecting packet discontinuity, handle_in_packet() returns minus value
and this value is assigned to unsigned int variable, then the variable has
huge value. As a result, the variable causes buffer-over-run in
handle_out_packet(). This brings invalid page request and system hangup.

This commit fixes the bug to add a new argument into handle_in_packet()
and the number of handled data blocks is assignd to it. The function
return value is just used to check error.

I also considered to change the type of local variable to 'int' in
in_stream_callback(). This idea is based on type-conversion in C standard,
while it may cause future problems when adding more works. Thus, I dropped
this idea.

Fixes: 6fc6b9ce41c6('ALSA: firewire-lib: pass the number of data blocks in incoming packets to outgoing packets')
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

authored by

Takashi Sakamoto and committed by
Takashi Iwai
31ea49ba 0d769a52

+16 -16
+16 -16
sound/firewire/amdtp.c
··· 688 688 } 689 689 690 690 static int handle_in_packet(struct amdtp_stream *s, 691 - unsigned int payload_quadlets, __be32 *buffer) 691 + unsigned int payload_quadlets, __be32 *buffer, 692 + unsigned int *data_blocks) 692 693 { 693 694 u32 cip_header[2]; 694 - unsigned int data_blocks; 695 695 unsigned int data_block_quadlets, data_block_counter, dbc_interval; 696 696 struct snd_pcm_substream *pcm = NULL; 697 697 bool lost; ··· 709 709 dev_info_ratelimited(&s->unit->device, 710 710 "Invalid CIP header for AMDTP: %08X:%08X\n", 711 711 cip_header[0], cip_header[1]); 712 - data_blocks = 0; 712 + *data_blocks = 0; 713 713 goto end; 714 714 } 715 715 ··· 717 717 if (payload_quadlets < 3 || 718 718 ((cip_header[1] & CIP_FDF_MASK) == 719 719 (AMDTP_FDF_NO_DATA << CIP_FDF_SHIFT))) { 720 - data_blocks = 0; 720 + *data_blocks = 0; 721 721 } else { 722 722 data_block_quadlets = 723 723 (cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT; ··· 731 731 if (s->flags & CIP_WRONG_DBS) 732 732 data_block_quadlets = s->data_block_quadlets; 733 733 734 - data_blocks = (payload_quadlets - 2) / data_block_quadlets; 734 + *data_blocks = (payload_quadlets - 2) / data_block_quadlets; 735 735 } 736 736 737 737 /* Check data block counter continuity */ 738 738 data_block_counter = cip_header[0] & CIP_DBC_MASK; 739 - if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) && 739 + if (*data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) && 740 740 s->data_block_counter != UINT_MAX) 741 741 data_block_counter = s->data_block_counter; 742 742 ··· 746 746 } else if (!(s->flags & CIP_DBC_IS_END_EVENT)) { 747 747 lost = data_block_counter != s->data_block_counter; 748 748 } else { 749 - if ((data_blocks > 0) && (s->tx_dbc_interval > 0)) 749 + if ((*data_blocks > 0) && (s->tx_dbc_interval > 0)) 750 750 dbc_interval = s->tx_dbc_interval; 751 751 else 752 - dbc_interval = data_blocks; 752 + dbc_interval = *data_blocks; 753 753 754 754 lost = data_block_counter != 755 755 ((s->data_block_counter + dbc_interval) & 0xff); ··· 762 762 return -EIO; 763 763 } 764 764 765 - if (data_blocks > 0) { 765 + if (*data_blocks > 0) { 766 766 buffer += 2; 767 767 768 768 pcm = ACCESS_ONCE(s->pcm); 769 769 if (pcm) 770 - s->transfer_samples(s, pcm, buffer, data_blocks); 770 + s->transfer_samples(s, pcm, buffer, *data_blocks); 771 771 772 772 if (s->midi_ports) 773 - read_midi_messages(s, buffer, data_blocks); 773 + read_midi_messages(s, buffer, *data_blocks); 774 774 } 775 775 776 776 if (s->flags & CIP_DBC_IS_END_EVENT) 777 777 s->data_block_counter = data_block_counter; 778 778 else 779 779 s->data_block_counter = 780 - (data_block_counter + data_blocks) & 0xff; 780 + (data_block_counter + *data_blocks) & 0xff; 781 781 end: 782 782 if (queue_in_packet(s) < 0) 783 783 return -EIO; 784 784 785 785 if (pcm) 786 - update_pcm_pointers(s, pcm, data_blocks); 786 + update_pcm_pointers(s, pcm, *data_blocks); 787 787 788 - return data_blocks; 788 + return 0; 789 789 } 790 790 791 791 static void out_stream_callback(struct fw_iso_context *context, u32 cycle, ··· 853 853 break; 854 854 } 855 855 856 - data_blocks = handle_in_packet(s, payload_quadlets, buffer); 857 - if (data_blocks < 0) { 856 + if (handle_in_packet(s, payload_quadlets, buffer, 857 + &data_blocks) < 0) { 858 858 s->packet_index = -1; 859 859 break; 860 860 }