On 10/12/2021 22:10, Péter Ujfalusi wrote:
>
>
> On 09/12/2021 20:09, Vignesh Raghavendra wrote:
>> Smatch reports below warnings [1] wrt dereferencing rm_res when it can
>> potentially be ERR_PTR(). This is possible when entire range is
>> allocated to Linux
>> Fix this case by making sure, there is no deference of rm_res when its
>> ERR_PTR().
>
> Valid, early sysfs did not had rm ranges, thus we assumed all channels
s/sysfs/sysfw
> are for Linux, then we got support for one range per channel type and
> then the sets got introduced for supporting the differnt throughput
> levels of channles in the types.
> This surely got overlooked.
>
> Acked-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxx>
>
>>
>> [1]:
>> drivers/dma/ti/k3-udma.c:4524 udma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4537 udma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4681 bcdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4696 bcdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4711 bcdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4848 pktdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4861 pktdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>>
>> Reported-by: Nishanth Menon <nm@xxxxxx>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@xxxxxx>
>> ---
>> drivers/dma/ti/k3-udma.c | 157 ++++++++++++++++++++++++++-------------
>> 1 file changed, 107 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index 041d8e32d630..6e56d1cef5ee 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -4534,45 +4534,60 @@ static int udma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->tchan_map, ud->tchan_cnt);
>> + irq_res.sets = 1;
>> } else {
>> bitmap_fill(ud->tchan_map, ud->tchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->tchan_map,
>> &rm_res->desc[i], "tchan");
>> + irq_res.sets = rm_res->sets;
>> }
>> - irq_res.sets = rm_res->sets;
>>
>> /* rchan and matching default flow ranges */
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->rchan_map, ud->rchan_cnt);
>> + irq_res.sets++;
>> } else {
>> bitmap_fill(ud->rchan_map, ud->rchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->rchan_map,
>> &rm_res->desc[i], "rchan");
>> + irq_res.sets += rm_res->sets;
>> }
>>
>> - irq_res.sets += rm_res->sets;
>> irq_res.desc = kcalloc(irq_res.sets, sizeof(*irq_res.desc), GFP_KERNEL);
>> + if (!irq_res.desc)
>> + return -ENOMEM;
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> - for (i = 0; i < rm_res->sets; i++) {
>> - irq_res.desc[i].start = rm_res->desc[i].start;
>> - irq_res.desc[i].num = rm_res->desc[i].num;
>> - irq_res.desc[i].start_sec = rm_res->desc[i].start_sec;
>> - irq_res.desc[i].num_sec = rm_res->desc[i].num_sec;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[0].start = 0;
>> + irq_res.desc[0].num = ud->tchan_cnt;
>> + i = 1;
>> + } else {
>> + for (i = 0; i < rm_res->sets; i++) {
>> + irq_res.desc[i].start = rm_res->desc[i].start;
>> + irq_res.desc[i].num = rm_res->desc[i].num;
>> + irq_res.desc[i].start_sec = rm_res->desc[i].start_sec;
>> + irq_res.desc[i].num_sec = rm_res->desc[i].num_sec;
>> + }
>> }
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> - for (j = 0; j < rm_res->sets; j++, i++) {
>> - if (rm_res->desc[j].num) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - ud->soc_data->oes.udma_rchan;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> - }
>> - if (rm_res->desc[j].num_sec) {
>> - irq_res.desc[i].start_sec = rm_res->desc[j].start_sec +
>> - ud->soc_data->oes.udma_rchan;
>> - irq_res.desc[i].num_sec = rm_res->desc[j].num_sec;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = 0;
>> + irq_res.desc[i].num = ud->rchan_cnt;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i++) {
>> + if (rm_res->desc[j].num) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + ud->soc_data->oes.udma_rchan;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> + }
>> + if (rm_res->desc[j].num_sec) {
>> + irq_res.desc[i].start_sec = rm_res->desc[j].start_sec +
>> + ud->soc_data->oes.udma_rchan;
>> + irq_res.desc[i].num_sec = rm_res->desc[j].num_sec;
>> + }
>> }
>> }
>> ret = ti_sci_inta_msi_domain_alloc_irqs(ud->dev, &irq_res);
>> @@ -4690,14 +4705,15 @@ static int bcdma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_BCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->bchan_map, ud->bchan_cnt);
>> + irq_res.sets++;
>> } else {
>> bitmap_fill(ud->bchan_map, ud->bchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->bchan_map,
>> &rm_res->desc[i],
>> "bchan");
>> + irq_res.sets += rm_res->sets;
>> }
>> - irq_res.sets += rm_res->sets;
>> }
>>
>> /* tchan ranges */
>> @@ -4705,14 +4721,15 @@ static int bcdma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->tchan_map, ud->tchan_cnt);
>> + irq_res.sets += 2;
>> } else {
>> bitmap_fill(ud->tchan_map, ud->tchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->tchan_map,
>> &rm_res->desc[i],
>> "tchan");
>> + irq_res.sets += rm_res->sets * 2;
>> }
>> - irq_res.sets += rm_res->sets * 2;
>> }
>>
>> /* rchan ranges */
>> @@ -4720,47 +4737,72 @@ static int bcdma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->rchan_map, ud->rchan_cnt);
>> + irq_res.sets += 2;
>> } else {
>> bitmap_fill(ud->rchan_map, ud->rchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->rchan_map,
>> &rm_res->desc[i],
>> "rchan");
>> + irq_res.sets += rm_res->sets * 2;
>> }
>> - irq_res.sets += rm_res->sets * 2;
>> }
>>
>> irq_res.desc = kcalloc(irq_res.sets, sizeof(*irq_res.desc), GFP_KERNEL);
>> + if (!irq_res.desc)
>> + return -ENOMEM;
>> if (ud->bchan_cnt) {
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_BCHAN];
>> - for (i = 0; i < rm_res->sets; i++) {
>> - irq_res.desc[i].start = rm_res->desc[i].start +
>> - oes->bcdma_bchan_ring;
>> - irq_res.desc[i].num = rm_res->desc[i].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[0].start = oes->bcdma_bchan_ring;
>> + irq_res.desc[0].num = ud->bchan_cnt;
>> + i = 1;
>> + } else {
>> + for (i = 0; i < rm_res->sets; i++) {
>> + irq_res.desc[i].start = rm_res->desc[i].start +
>> + oes->bcdma_bchan_ring;
>> + irq_res.desc[i].num = rm_res->desc[i].num;
>> + }
>> }
>> }
>> if (ud->tchan_cnt) {
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> - for (j = 0; j < rm_res->sets; j++, i += 2) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - oes->bcdma_tchan_data;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> -
>> - irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> - oes->bcdma_tchan_ring;
>> - irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = oes->bcdma_tchan_data;
>> + irq_res.desc[i].num = ud->tchan_cnt;
>> + irq_res.desc[i + 1].start = oes->bcdma_tchan_ring;
>> + irq_res.desc[i + 1].num = ud->tchan_cnt;
>> + i += 2;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i += 2) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + oes->bcdma_tchan_data;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> +
>> + irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> + oes->bcdma_tchan_ring;
>> + irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + }
>> }
>> }
>> if (ud->rchan_cnt) {
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> - for (j = 0; j < rm_res->sets; j++, i += 2) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - oes->bcdma_rchan_data;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> -
>> - irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> - oes->bcdma_rchan_ring;
>> - irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = oes->bcdma_rchan_data;
>> + irq_res.desc[i].num = ud->rchan_cnt;
>> + irq_res.desc[i + 1].start = oes->bcdma_rchan_ring;
>> + irq_res.desc[i + 1].num = ud->rchan_cnt;
>> + i += 2;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i += 2) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + oes->bcdma_rchan_data;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> +
>> + irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> + oes->bcdma_rchan_ring;
>> + irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + }
>> }
>> }
>>
>> @@ -4858,39 +4900,54 @@ static int pktdma_setup_resources(struct udma_dev *ud)
>> if (IS_ERR(rm_res)) {
>> /* all rflows are assigned exclusively to Linux */
>> bitmap_zero(ud->rflow_in_use, ud->rflow_cnt);
>> + irq_res.sets = 1;
>> } else {
>> bitmap_fill(ud->rflow_in_use, ud->rflow_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->rflow_in_use,
>> &rm_res->desc[i], "rflow");
>> + irq_res.sets = rm_res->sets;
>> }
>> - irq_res.sets = rm_res->sets;
>>
>> /* tflow ranges */
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TFLOW];
>> if (IS_ERR(rm_res)) {
>> /* all tflows are assigned exclusively to Linux */
>> bitmap_zero(ud->tflow_map, ud->tflow_cnt);
>> + irq_res.sets++;
>> } else {
>> bitmap_fill(ud->tflow_map, ud->tflow_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->tflow_map,
>> &rm_res->desc[i], "tflow");
>> + irq_res.sets += rm_res->sets;
>> }
>> - irq_res.sets += rm_res->sets;
>>
>> irq_res.desc = kcalloc(irq_res.sets, sizeof(*irq_res.desc), GFP_KERNEL);
>> + if (!irq_res.desc)
>> + return -ENOMEM;
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TFLOW];
>> - for (i = 0; i < rm_res->sets; i++) {
>> - irq_res.desc[i].start = rm_res->desc[i].start +
>> - oes->pktdma_tchan_flow;
>> - irq_res.desc[i].num = rm_res->desc[i].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[0].start = oes->pktdma_tchan_flow;
>> + irq_res.desc[0].num = ud->tflow_cnt;
>> + i = 1;
>> + } else {
>> + for (i = 0; i < rm_res->sets; i++) {
>> + irq_res.desc[i].start = rm_res->desc[i].start +
>> + oes->pktdma_tchan_flow;
>> + irq_res.desc[i].num = rm_res->desc[i].num;
>> + }
>> }
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RFLOW];
>> - for (j = 0; j < rm_res->sets; j++, i++) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - oes->pktdma_rchan_flow;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = oes->pktdma_rchan_flow;
>> + irq_res.desc[i].num = ud->rflow_cnt;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i++) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + oes->pktdma_rchan_flow;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> + }
>> }
>> ret = ti_sci_inta_msi_domain_alloc_irqs(ud->dev, &irq_res);
>> kfree(irq_res.desc);
>>
>
--
Péter
