On Fri, 07 May 2021 11:18:56 +0200, Подгорный Алексей Олегович wrote:

snd_korg1212_create() makes the following steps during initialization of the card:

1. registers an interrupt handler (lines 2230-2232)
2. allocates and initializes korg1212->sharedBufferPtr (lines 2280-2287)
3. reboots the card via snd_korg1212_Send1212Command() (line 2358)

2145 static int snd_korg1212_create(struct snd_card *card, struct pci_dev *pci, struct snd_korg1212 **rchip) 2147 { ... 2230 err = request_irq(pci->irq, snd_korg1212_interrupt, 2231 IRQF_SHARED, 2232 KBUILD_MODNAME, korg1212); ... 2280 if (snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, &pci->dev, 2281 sizeof(struct KorgSharedBuffer), &korg1212->dma_shared) < 0){

2282 snd_printk(KERN_ERR "korg1212: can not allocate shared buffer memory (%zdbytes)\n", sizeof(struct KorgSharedBuffer));

2283 snd_korg1212_free(korg1212); 2284 return -ENOMEM; 2285 } 2286 korg1212->sharedBufferPtr = (struct KorgSharedBuffer*)korg1212->dma_shared.area; 2287 korg1212->sharedBufferPhy = korg1212->dma_shared.addr; ... 2358 rc = snd_korg1212_Send1212Command(korg1212, K1212_DB_RebootCard, 0, 0, 0, 0); ... 2412 }

But if interrupt happens when snd_korg1212_create() is still within lines 2233-2286, snd_korg1212_interrupt() may dereference korg1212->sharedBufferPtr before it was initialized without any checks (line 1149):

1098 static irqreturn_t snd_korg1212_interrupt(int irq, void *dev_id) 1099 { ... 1116 switch (doorbellValue) { ... 1145 case K1212_DB_CARDSTOPPED: 1146 K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: IRQ CSTP count - %ld, %x, [%s].\n", 1147 korg1212->irqcount, doorbellValue, 1148 stateName[korg1212->cardState]); 1149 korg1212->sharedBufferPtr->cardCommand = 0; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1150 break; ... 1185 }

Should we be sure that such interrupt cannot happen or should we move the registration of the interrupt handler after korg1212->sharedBufferPtr is initialized?

Yes, in general the IRQ handler should be registered at the end. Could you submit a fix patch?

thanks,

Takashi