* [bug report] Linux-2.6.12-rc2
@ 2025-05-28 8:10 Dan Carpenter
2025-05-30 5:46 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2025-05-28 8:10 UTC (permalink / raw)
To: linux-alpha; +Cc: Arnd Bergmann
Hello Alpha Maintainers,
Commit 1da177e4c3f4 ("Linux-2.6.12-rc2") from Apr 16, 2005
(linux-next), leads to the following Smatch static checker warning:
arch/alpha/kernel/err_marvel.c:884 marvel_find_io7_with_error() warn: statement has no effect 'csrs->POx_ERR_SUM.csr'
arch/alpha/kernel/err_marvel.c:892 marvel_find_io7_with_error() warn: statement has no effect 'io7->csrs->PO7_ERROR_SUM.csr'
arch/alpha/kernel/err_marvel.c
797 static struct ev7_pal_io_subpacket *
798 marvel_find_io7_with_error(struct ev7_lf_subpackets *lf_subpackets)
799 {
800 struct ev7_pal_io_subpacket *io = lf_subpackets->io;
801 struct io7 *io7;
802 int i;
803
804 /*
805 * Caller must provide the packet to fill
806 */
807 if (!io)
808 return NULL;
809
810 /*
811 * Fill the subpacket with the console's standard fill pattern
812 */
813 memset(io, 0x55, sizeof(*io));
814
815 for (io7 = NULL; NULL != (io7 = marvel_next_io7(io7)); ) {
816 unsigned long err_sum = 0;
817
818 err_sum |= io7->csrs->PO7_ERROR_SUM.csr;
819 for (i = 0; i < IO7_NUM_PORTS; i++) {
820 if (!io7->ports[i].enabled)
821 continue;
822 err_sum |= io7->ports[i].csrs->POx_ERR_SUM.csr;
823 }
824
825 /*
826 * Is there at least one error?
827 */
828 if (err_sum & (1UL << 63))
829 break;
830 }
831
832 /*
833 * Did we find an IO7 with an error?
834 */
835 if (!io7)
836 return NULL;
837
838 /*
839 * We have an IO7 with an error.
840 *
841 * Fill in the IO subpacket.
842 */
843 io->io_asic_rev = io7->csrs->IO_ASIC_REV.csr;
844 io->io_sys_rev = io7->csrs->IO_SYS_REV.csr;
845 io->io7_uph = io7->csrs->IO7_UPH.csr;
846 io->hpi_ctl = io7->csrs->HPI_CTL.csr;
847 io->crd_ctl = io7->csrs->CRD_CTL.csr;
848 io->hei_ctl = io7->csrs->HEI_CTL.csr;
849 io->po7_error_sum = io7->csrs->PO7_ERROR_SUM.csr;
850 io->po7_uncrr_sym = io7->csrs->PO7_UNCRR_SYM.csr;
851 io->po7_crrct_sym = io7->csrs->PO7_CRRCT_SYM.csr;
852 io->po7_ugbge_sym = io7->csrs->PO7_UGBGE_SYM.csr;
853 io->po7_err_pkt0 = io7->csrs->PO7_ERR_PKT[0].csr;
854 io->po7_err_pkt1 = io7->csrs->PO7_ERR_PKT[1].csr;
855
856 for (i = 0; i < IO7_NUM_PORTS; i++) {
857 io7_ioport_csrs *csrs = io7->ports[i].csrs;
858
859 if (!io7->ports[i].enabled)
860 continue;
861
862 io->ports[i].pox_err_sum = csrs->POx_ERR_SUM.csr;
863 io->ports[i].pox_tlb_err = csrs->POx_TLB_ERR.csr;
864 io->ports[i].pox_spl_cmplt = csrs->POx_SPL_COMPLT.csr;
865 io->ports[i].pox_trans_sum = csrs->POx_TRANS_SUM.csr;
866 io->ports[i].pox_first_err = csrs->POx_FIRST_ERR.csr;
867 io->ports[i].pox_mult_err = csrs->POx_MULT_ERR.csr;
868 io->ports[i].pox_dm_source = csrs->POx_DM_SOURCE.csr;
869 io->ports[i].pox_dm_dest = csrs->POx_DM_DEST.csr;
870 io->ports[i].pox_dm_size = csrs->POx_DM_SIZE.csr;
871 io->ports[i].pox_dm_ctrl = csrs->POx_DM_CTRL.csr;
872
873 /*
874 * Ack this port's errors, if any. POx_ERR_SUM must be last.
875 *
876 * Most of the error registers get cleared and unlocked when
877 * the associated bits in POx_ERR_SUM are cleared (by writing
878 * 1). POx_TLB_ERR is an exception and must be explicitly
879 * cleared.
880 */
881 csrs->POx_TLB_ERR.csr = io->ports[i].pox_tlb_err;
882 csrs->POx_ERR_SUM.csr = io->ports[i].pox_err_sum;
883 mb();
--> 884 csrs->POx_ERR_SUM.csr;
^^^^^^^^^^^^^^^^^^^^^^
I occasionally try to run Smatch against code that I can't actually
compile and it found this code from before the git era. These days
we build with -Wall and so this kind of code doesn't normally compile
on x86 or Arm. We would get a warning like:
test.c:18:9: warning: statement with no effect [-Wunused-value]
18 | x;
| ^
So it makes me wonder if this code can actually build?
885 }
886
887 /*
888 * Ack any port 7 error(s).
889 */
890 io7->csrs->PO7_ERROR_SUM.csr = io->po7_error_sum;
91 mb();
--> 892 io7->csrs->PO7_ERROR_SUM.csr;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
893
894 /*
895 * Correct the io7_pid.
896 */
897 lf_subpackets->io_pid = io7->pe;
898
899 return io;
900 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] Linux-2.6.12-rc2
2025-05-28 8:10 [bug report] Linux-2.6.12-rc2 Dan Carpenter
@ 2025-05-30 5:46 ` Arnd Bergmann
2025-06-02 7:33 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2025-05-30 5:46 UTC (permalink / raw)
To: Dan Carpenter, linux-alpha
On Wed, May 28, 2025, at 10:10, Dan Carpenter wrote:
>
> arch/alpha/kernel/err_marvel.c:884 marvel_find_io7_with_error() warn:
> statement has no effect 'csrs->POx_ERR_SUM.csr'
> arch/alpha/kernel/err_marvel.c:892 marvel_find_io7_with_error() warn:
> statement has no effect 'io7->csrs->PO7_ERROR_SUM.csr'
>
> I occasionally try to run Smatch against code that I can't actually
> compile and it found this code from before the git era. These days
> we build with -Wall and so this kind of code doesn't normally compile
> on x86 or Arm. We would get a warning like:
>
> test.c:18:9: warning: statement with no effect [-Wunused-value]
> 18 | x;
> | ^
>
> So it makes me wonder if this code can actually build?
>
FWIW, I tried building the file with gcc-15 and don't see a
warning for that construct, even with the -Wextra.
After digging around some more, I found that this is the
definition of the structure, and that the 'volatile' in there
causes gcc and clang to not consider the statement to be
free of side-effects. I assume it will actually cause a
load from an MMIO register here:
typedef struct {
volatile unsigned long csr __attribute__((aligned(64)));
} io7_csr;
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] Linux-2.6.12-rc2
2025-05-30 5:46 ` Arnd Bergmann
@ 2025-06-02 7:33 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2025-06-02 7:33 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-alpha
On Fri, May 30, 2025 at 07:46:20AM +0200, Arnd Bergmann wrote:
> On Wed, May 28, 2025, at 10:10, Dan Carpenter wrote:
> >
> > arch/alpha/kernel/err_marvel.c:884 marvel_find_io7_with_error() warn:
> > statement has no effect 'csrs->POx_ERR_SUM.csr'
> > arch/alpha/kernel/err_marvel.c:892 marvel_find_io7_with_error() warn:
> > statement has no effect 'io7->csrs->PO7_ERROR_SUM.csr'
> >
>
> > I occasionally try to run Smatch against code that I can't actually
> > compile and it found this code from before the git era. These days
> > we build with -Wall and so this kind of code doesn't normally compile
> > on x86 or Arm. We would get a warning like:
> >
> > test.c:18:9: warning: statement with no effect [-Wunused-value]
> > 18 | x;
> > | ^
> >
> > So it makes me wonder if this code can actually build?
> >
>
> FWIW, I tried building the file with gcc-15 and don't see a
> warning for that construct, even with the -Wextra.
>
> After digging around some more, I found that this is the
> definition of the structure, and that the 'volatile' in there
> causes gcc and clang to not consider the statement to be
> free of side-effects. I assume it will actually cause a
> load from an MMIO register here:
>
> typedef struct {
> volatile unsigned long csr __attribute__((aligned(64)));
> } io7_csr;
Huh. Thanks, Arnd. I hadn't even considered that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug report] Linux-2.6.12-rc2
@ 2017-10-17 21:52 Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-10-17 21:52 UTC (permalink / raw)
To: Adaptec OEM Raid Solutions; +Cc: linux-scsi
Hey,
This code is older than git is so it probably doesn't matter. But just
for laughs does anyone know what this should be?
drivers/scsi/aic7xxx/aic7xxx_core.c:4807 ahc_init_scbdata()
warn: integer overflow (literal): u32max + 1
drivers/scsi/aic7xxx/aic7xxx_core.c
4794
4795 /*
4796 * Create our DMA tags. These tags define the kinds of device
4797 * accessible memory allocations and memory mappings we will
4798 * need to perform during normal operation.
4799 *
4800 * Unless we need to further restrict the allocation, we rely
4801 * on the restrictions of the parent dmat, hence the common
4802 * use of MAXADDR and MAXSIZE.
4803 */
4804
4805 /* DMA tag for our hardware scb structures */
4806 if (ahc_dma_tag_create(ahc, ahc->parent_dmat, /*alignment*/1,
4807 /*boundary*/BUS_SPACE_MAXADDR_32BIT + 1,
^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is "0xffffffff + 1" which has an integer overflow so it's a
complicated way to say zero.
4808 /*lowaddr*/BUS_SPACE_MAXADDR_32BIT,
4809 /*highaddr*/BUS_SPACE_MAXADDR,
4810 /*filter*/NULL, /*filterarg*/NULL,
4811 AHC_SCB_MAX_ALLOC * sizeof(struct hardware_scb),
4812 /*nsegments*/1,
4813 /*maxsegsz*/BUS_SPACE_MAXSIZE_32BIT,
4814 /*flags*/0, &scb_data->hscb_dmat) != 0) {
4815 goto error_exit;
4816 }
4817
4818 scb_data->init_level++;
4819
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* [bug report] Linux-2.6.12-rc2
@ 2016-07-14 22:22 Dan Carpenter
2016-07-15 3:24 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2016-07-14 22:22 UTC (permalink / raw)
To: linuxppc-dev
Hi PPC Devs,
The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
to the following static checker warning:
arch/powerpc/sysdev/ipic.c:783 ipic_set_priority()
error: buffer overflow 'ipic_info' 95 <= 127
arch/powerpc/sysdev/ipic.c
36 static struct ipic_info ipic_info[] = {
37 [1] = {
38 .mask = IPIC_SIMSR_H,
39 .prio = IPIC_SIPRR_C,
40 .force = IPIC_SIFCR_H,
41 .bit = 16,
42 .prio_mask = 0,
43 },
[ huge 95 element array snipped ]
500 [94] = {
501 .mask = IPIC_SIMSR_L,
502 .prio = 0,
503 .force = IPIC_SIFCR_L,
504 .bit = 30,
505 },
506 };
[ more code snipped ]
773 int ipic_set_priority(unsigned int virq, unsigned int priority)
774 {
775 struct ipic *ipic = ipic_from_irq(virq);
776 unsigned int src = virq_to_hw(virq);
777 u32 temp;
778
779 if (priority > 7)
780 return -EINVAL;
781 if (src > 127)
^^^^^^^^^
We cap this at 127
782 return -EINVAL;
783 if (ipic_info[src].prio == 0)
^^^^^^^^^^^^^^
But we only have 95 elements. Should the array be larger or should
we >= ARRAY_SIZE(ipic_info) is invalid?
784 return -EINVAL;
785
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] Linux-2.6.12-rc2
2016-07-14 22:22 Dan Carpenter
@ 2016-07-15 3:24 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-07-15 3:24 UTC (permalink / raw)
To: Dan Carpenter, linuxppc-dev
Dan Carpenter <dan.carpenter@oracle.com> writes:
> Hi PPC Devs,
>
> The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
Might want to special case that one :)
> to the following static checker warning:
>
> arch/powerpc/sysdev/ipic.c:783 ipic_set_priority()
> error: buffer overflow 'ipic_info' 95 <= 127
...
>
> 773 int ipic_set_priority(unsigned int virq, unsigned int priority)
> 774 {
> 775 struct ipic *ipic = ipic_from_irq(virq);
> 776 unsigned int src = virq_to_hw(virq);
> 777 u32 temp;
> 778
> 779 if (priority > 7)
> 780 return -EINVAL;
> 781 if (src > 127)
> ^^^^^^^^^
> We cap this at 127
>
> 782 return -EINVAL;
> 783 if (ipic_info[src].prio == 0)
> ^^^^^^^^^^^^^^
> But we only have 95 elements. Should the array be larger or should
> we >= ARRAY_SIZE(ipic_info) is invalid?
I don't know the code personally, but looking at the history it seems new
interrupts are added manually with specific flags.
So testing against ARRAY_SIZE would be the best fix AFAICS.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-02 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28 8:10 [bug report] Linux-2.6.12-rc2 Dan Carpenter
2025-05-30 5:46 ` Arnd Bergmann
2025-06-02 7:33 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2017-10-17 21:52 Dan Carpenter
2016-07-14 22:22 Dan Carpenter
2016-07-15 3:24 ` Michael Ellerman
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.