* [bug report] ALSA: usb: add UAC3 BADD profiles support
@ 2018-05-16 11:32 Dan Carpenter
2018-05-16 11:36 ` Ruslan Bilovol
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2018-05-16 11:32 UTC (permalink / raw)
To: ruslan.bilovol; +Cc: alsa-devel
[ Your patch didn't really introduce the bug, it just made it visible
to static checkers. - dan ]
Hello Ruslan Bilovol,
The patch 17156f23e93c: "ALSA: usb: add UAC3 BADD profiles support"
from May 4, 2018, leads to the following static checker warning:
sound/usb/stream.c:984 snd_usb_get_audioformat_uac3()
warn: possible memory leak of 'chmap'
sound/usb/stream.c
864
865 chmap = kzalloc(sizeof(*chmap), GFP_KERNEL);
866 if (!chmap)
867 return ERR_PTR(-ENOMEM);
868
869 if (num_channels == 1) {
870 chmap->map[0] = SNDRV_CHMAP_MONO;
871 } else {
872 chmap->map[0] = SNDRV_CHMAP_FL;
873 chmap->map[1] = SNDRV_CHMAP_FR;
874 }
875
876 chmap->channels = num_channels;
877 clock = UAC3_BADD_CS_ID9;
878 goto found_clock;
879 }
880
881 as = snd_usb_find_csint_desc(alts->extra, alts->extralen,
882 NULL, UAC_AS_GENERAL);
883 if (!as) {
884 dev_err(&dev->dev,
885 "%u:%d : UAC_AS_GENERAL descriptor not found\n",
886 iface_no, altno);
887 return NULL;
888 }
889
890 if (as->bLength < sizeof(*as)) {
891 dev_err(&dev->dev,
892 "%u:%d : invalid UAC_AS_GENERAL desc\n",
893 iface_no, altno);
894 return NULL;
895 }
896
897 cluster_id = le16_to_cpu(as->wClusterDescrID);
898 if (!cluster_id) {
899 dev_err(&dev->dev,
900 "%u:%d : no cluster descriptor\n",
901 iface_no, altno);
902 return NULL;
903 }
904
905 /*
906 * Get number of channels and channel map through
907 * High Capability Cluster Descriptor
908 *
909 * First step: get High Capability header and
910 * read size of Cluster Descriptor
911 */
912 err = snd_usb_ctl_msg(chip->dev,
913 usb_rcvctrlpipe(chip->dev, 0),
914 UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
915 USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
916 cluster_id,
917 snd_usb_ctrl_intf(chip),
918 &hc_header, sizeof(hc_header));
919 if (err < 0)
920 return ERR_PTR(err);
921 else if (err != sizeof(hc_header)) {
922 dev_err(&dev->dev,
923 "%u:%d : can't get High Capability descriptor\n",
924 iface_no, altno);
925 return ERR_PTR(-EIO);
926 }
927
928 /*
929 * Second step: allocate needed amount of memory
930 * and request Cluster Descriptor
931 */
932 wLength = le16_to_cpu(hc_header.wLength);
933 cluster = kzalloc(wLength, GFP_KERNEL);
934 if (!cluster)
935 return ERR_PTR(-ENOMEM);
936 err = snd_usb_ctl_msg(chip->dev,
937 usb_rcvctrlpipe(chip->dev, 0),
938 UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
939 USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
940 cluster_id,
941 snd_usb_ctrl_intf(chip),
942 cluster, wLength);
943 if (err < 0) {
944 kfree(cluster);
945 return ERR_PTR(err);
946 } else if (err != wLength) {
947 dev_err(&dev->dev,
948 "%u:%d : can't get Cluster Descriptor\n",
949 iface_no, altno);
950 kfree(cluster);
951 return ERR_PTR(-EIO);
952 }
953
954 num_channels = cluster->bNrChannels;
955 chmap = convert_chmap_v3(cluster);
956 kfree(cluster);
957
958 /*
959 * lookup the terminal associated to this interface
960 * to extract the clock
961 */
962 input_term = snd_usb_find_input_terminal_descriptor(chip->ctrl_intf,
963 as->bTerminalLink);
964 if (input_term) {
965 clock = input_term->bCSourceID;
966 goto found_clock;
967 }
968
969 output_term = snd_usb_find_output_terminal_descriptor(chip->ctrl_intf,
970 as->bTerminalLink);
971 if (output_term) {
972 clock = output_term->bCSourceID;
973 goto found_clock;
974 }
975
976 dev_err(&dev->dev, "%u:%d : bogus bTerminalLink %d\n",
977 iface_no, altno, as->bTerminalLink);
978 return NULL;
979
980 found_clock:
981 fp = audio_format_alloc_init(chip, alts, UAC_VERSION_3, iface_no,
982 altset_idx, altno, num_channels, clock);
983 if (!fp)
984 return ERR_PTR(-ENOMEM);
We should free "chmap" before returning.
985
986 fp->chmap = chmap;
987
988 if (badd_profile >= UAC3_FUNCTION_SUBCLASS_GENERIC_IO) {
989 fp->attributes = 0; /* No attributes */
990
991 fp->fmt_type = UAC_FORMAT_TYPE_I;
992 fp->formats = badd_formats;
993
994 fp->nr_rates = 0; /* SNDRV_PCM_RATE_CONTINUOUS */
995 fp->rate_min = UAC3_BADD_SAMPLING_RATE;
996 fp->rate_max = UAC3_BADD_SAMPLING_RATE;
997 fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
998
999 } else {
1000 fp->attributes = parse_uac_endpoint_attributes(chip, alts,
1001 UAC_VERSION_3,
1002 iface_no);
1003 /* ok, let's parse further... */
1004 if (snd_usb_parse_audio_format_v3(chip, fp, as, stream) < 0) {
1005 kfree(fp->rate_table);
1006 kfree(fp);
1007 return NULL;
And here as well? I don't know.
1008 }
1009 }
1010
1011 return fp;
1012 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] ALSA: usb: add UAC3 BADD profiles support
2018-05-16 11:32 [bug report] ALSA: usb: add UAC3 BADD profiles support Dan Carpenter
@ 2018-05-16 11:36 ` Ruslan Bilovol
0 siblings, 0 replies; 2+ messages in thread
From: Ruslan Bilovol @ 2018-05-16 11:36 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel
Hi Dan,
On Wed, May 16, 2018 at 2:32 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> [ Your patch didn't really introduce the bug, it just made it visible
> to static checkers. - dan ]
>
> Hello Ruslan Bilovol,
>
> The patch 17156f23e93c: "ALSA: usb: add UAC3 BADD profiles support"
> from May 4, 2018, leads to the following static checker warning:
>
> sound/usb/stream.c:984 snd_usb_get_audioformat_uac3()
> warn: possible memory leak of 'chmap'
>
> sound/usb/stream.c
> 864
> 865 chmap = kzalloc(sizeof(*chmap), GFP_KERNEL);
> 866 if (!chmap)
> 867 return ERR_PTR(-ENOMEM);
> 868
> 869 if (num_channels == 1) {
> 870 chmap->map[0] = SNDRV_CHMAP_MONO;
> 871 } else {
> 872 chmap->map[0] = SNDRV_CHMAP_FL;
> 873 chmap->map[1] = SNDRV_CHMAP_FR;
> 874 }
> 875
> 876 chmap->channels = num_channels;
> 877 clock = UAC3_BADD_CS_ID9;
> 878 goto found_clock;
> 879 }
> 880
> 881 as = snd_usb_find_csint_desc(alts->extra, alts->extralen,
> 882 NULL, UAC_AS_GENERAL);
> 883 if (!as) {
> 884 dev_err(&dev->dev,
> 885 "%u:%d : UAC_AS_GENERAL descriptor not found\n",
> 886 iface_no, altno);
> 887 return NULL;
> 888 }
> 889
> 890 if (as->bLength < sizeof(*as)) {
> 891 dev_err(&dev->dev,
> 892 "%u:%d : invalid UAC_AS_GENERAL desc\n",
> 893 iface_no, altno);
> 894 return NULL;
> 895 }
> 896
> 897 cluster_id = le16_to_cpu(as->wClusterDescrID);
> 898 if (!cluster_id) {
> 899 dev_err(&dev->dev,
> 900 "%u:%d : no cluster descriptor\n",
> 901 iface_no, altno);
> 902 return NULL;
> 903 }
> 904
> 905 /*
> 906 * Get number of channels and channel map through
> 907 * High Capability Cluster Descriptor
> 908 *
> 909 * First step: get High Capability header and
> 910 * read size of Cluster Descriptor
> 911 */
> 912 err = snd_usb_ctl_msg(chip->dev,
> 913 usb_rcvctrlpipe(chip->dev, 0),
> 914 UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
> 915 USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
> 916 cluster_id,
> 917 snd_usb_ctrl_intf(chip),
> 918 &hc_header, sizeof(hc_header));
> 919 if (err < 0)
> 920 return ERR_PTR(err);
> 921 else if (err != sizeof(hc_header)) {
> 922 dev_err(&dev->dev,
> 923 "%u:%d : can't get High Capability descriptor\n",
> 924 iface_no, altno);
> 925 return ERR_PTR(-EIO);
> 926 }
> 927
> 928 /*
> 929 * Second step: allocate needed amount of memory
> 930 * and request Cluster Descriptor
> 931 */
> 932 wLength = le16_to_cpu(hc_header.wLength);
> 933 cluster = kzalloc(wLength, GFP_KERNEL);
> 934 if (!cluster)
> 935 return ERR_PTR(-ENOMEM);
> 936 err = snd_usb_ctl_msg(chip->dev,
> 937 usb_rcvctrlpipe(chip->dev, 0),
> 938 UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
> 939 USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
> 940 cluster_id,
> 941 snd_usb_ctrl_intf(chip),
> 942 cluster, wLength);
> 943 if (err < 0) {
> 944 kfree(cluster);
> 945 return ERR_PTR(err);
> 946 } else if (err != wLength) {
> 947 dev_err(&dev->dev,
> 948 "%u:%d : can't get Cluster Descriptor\n",
> 949 iface_no, altno);
> 950 kfree(cluster);
> 951 return ERR_PTR(-EIO);
> 952 }
> 953
> 954 num_channels = cluster->bNrChannels;
> 955 chmap = convert_chmap_v3(cluster);
> 956 kfree(cluster);
> 957
> 958 /*
> 959 * lookup the terminal associated to this interface
> 960 * to extract the clock
> 961 */
> 962 input_term = snd_usb_find_input_terminal_descriptor(chip->ctrl_intf,
> 963 as->bTerminalLink);
> 964 if (input_term) {
> 965 clock = input_term->bCSourceID;
> 966 goto found_clock;
> 967 }
> 968
> 969 output_term = snd_usb_find_output_terminal_descriptor(chip->ctrl_intf,
> 970 as->bTerminalLink);
> 971 if (output_term) {
> 972 clock = output_term->bCSourceID;
> 973 goto found_clock;
> 974 }
> 975
> 976 dev_err(&dev->dev, "%u:%d : bogus bTerminalLink %d\n",
> 977 iface_no, altno, as->bTerminalLink);
> 978 return NULL;
> 979
> 980 found_clock:
> 981 fp = audio_format_alloc_init(chip, alts, UAC_VERSION_3, iface_no,
> 982 altset_idx, altno, num_channels, clock);
> 983 if (!fp)
> 984 return ERR_PTR(-ENOMEM);
>
> We should free "chmap" before returning.
>
> 985
> 986 fp->chmap = chmap;
> 987
> 988 if (badd_profile >= UAC3_FUNCTION_SUBCLASS_GENERIC_IO) {
> 989 fp->attributes = 0; /* No attributes */
> 990
> 991 fp->fmt_type = UAC_FORMAT_TYPE_I;
> 992 fp->formats = badd_formats;
> 993
> 994 fp->nr_rates = 0; /* SNDRV_PCM_RATE_CONTINUOUS */
> 995 fp->rate_min = UAC3_BADD_SAMPLING_RATE;
> 996 fp->rate_max = UAC3_BADD_SAMPLING_RATE;
> 997 fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
> 998
> 999 } else {
> 1000 fp->attributes = parse_uac_endpoint_attributes(chip, alts,
> 1001 UAC_VERSION_3,
> 1002 iface_no);
> 1003 /* ok, let's parse further... */
> 1004 if (snd_usb_parse_audio_format_v3(chip, fp, as, stream) < 0) {
> 1005 kfree(fp->rate_table);
> 1006 kfree(fp);
> 1007 return NULL;
>
> And here as well? I don't know.
Correct, this was not obvious before UAC3 refactoring, but now the
issue is clear.
I'll send a fix soon.
Thanks,
Ruslan
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-05-16 11:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-16 11:32 [bug report] ALSA: usb: add UAC3 BADD profiles support Dan Carpenter
2018-05-16 11:36 ` Ruslan Bilovol
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.