coccinelle: Check for missing NULL terminators in of_device_id tables
PATCH v2 1/1 coccinelle: Check for missing NULL terminators in of_device_id tables
Failure to terminate an of_device_id table can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.
--- scripts/coccinelle/misc/of_table.cocci | 73 ++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 scripts/coccinelle/misc/of_table.cocci
scripts/coccinelle/misc/of_table.cocci new file +73 -0
@@ -0,0 +1,73 @@ +/// Make sure of_device_id tables are NULL terminated +// +// Keywords: of_table +// Confidence: Medium +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@bad_of_table exists@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, + } +}; + +@depends on bad_of_table && context@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { +* .var = E, + } +}; + +@depends on bad_of_table && patch@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, +- } ++ }, ++ { } +}; + +@r depends on bad_of_table exists@ +position p1; +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, + } + @p1 +}; + +@script:python depends on org@ +p1 << r.p1; +arr << r.arr; +@@ + +cocci.print_main(arr,p1) + +@script:python depends on report@ +p1 << r.p1; +arr << r.arr; +@@ + +msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line) +coccilib.report.print_report(p1[0],msg)
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 5 lines · Stephen Boyd
Failure to terminate an
of_device_idtable can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.
Quoted 8 lines
Cc Mitchel Humpherys <[email protected]>Cc Julia Lawall <[email protected]>Cc Gilles Muller <[email protected]>Cc Nicolas Palix <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>Signed-off-by Stephen Boyd <[email protected]>
Looks good to me:
Quoted 88 lines
--- scripts/coccinelle/misc/of_table.cocci | 73 ++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 scripts/coccinelle/misc/of_table.cocciscripts/coccinelle/misc/of_table.cocci new file +73
-0@@ -0,0 +1,73 @@ +/// Make sure of_device_id tables are NULL terminated +// +// Keywords: of_table +// Confidence: Medium +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@bad_of_table exists@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, + } +}; + +@depends on bad_of_table && context@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { +* .var = E, + } +}; + +@depends on bad_of_table && patch@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, +- } ++ }, ++ { } +}; + +@r depends on bad_of_table exists@ +position p1; +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, + } + @p1 +}; + +@script:python depends on org@ +p1 << r.p1; +arr << r.arr; +@@ + +cocci.print_main(arr,p1) + +@script:python depends on report@ +p1 << r.p1; +arr << r.arr; +@@ + +msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line) +coccilib.report.print_report(p1[0],msg)Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Failure to terminate an of_device_id table can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.
---
v3: Removed unneeded rule bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.
scripts/coccinelle/misc/of_table.cocci | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
scripts/coccinelle/misc/of_table.cocci new file +62 -0
@@ -0,0 +1,62 @@ +/// Make sure of_device_id tables are NULL terminated +// +// Keywords: of_table +// Confidence: Medium +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@depends on context@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, +* } +}; + +@depends on patch@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, +- } ++ }, ++ { } +}; + +@r depends on org || report@ +position p1; +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, + } + @p1 +}; + +@script:python depends on org@ +p1 << r.p1; +arr << r.arr; +@@ + +cocci.print_main(arr,p1) + +@script:python depends on report@ +p1 << r.p1; +arr << r.arr; +@@ + +msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line) +coccilib.report.print_report(p1[0],msg)
Quoted 16 lines · Julia Lawall
Failure to terminate an
of_device_idtable can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.Cc Mitchel Humpherys <[email protected]>Cc Julia Lawall <[email protected]>Cc Gilles Muller <[email protected]>Cc Nicolas Palix <nicolas.palix-wMUr/[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>Signed-off-by Stephen Boyd <[email protected]>Acked-by Grant Likely <[email protected]>Signed-off-by Julia Lawall <[email protected]>
Quoted 5 lines
---
v3: Removed unneeded rule
bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.
These seem like only minor changes. Why was my authorship removed?
Signature · 7 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 17 lines · Stephen Boyd
Quoted 16 lines · Julia Lawall
Failure to terminate an
of_device_idtable can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.Cc Mitchel Humpherys <[email protected]>Cc Julia Lawall <[email protected]>Cc Gilles Muller <[email protected]>Cc Nicolas Palix <nicolas.palix-wMUr/[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>Signed-off-by Stephen Boyd <[email protected]>Acked-by Grant Likely <[email protected]>Signed-off-by Julia Lawall <[email protected]>
Quoted 8 lines
Quoted 5 lines
---
v3: Removed unneeded rule
bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.These seem like only minor changes. Why was my authorship removed?
Probably because I don't know how to provide a new vesion of the patch properly. What should I have done? There were quite a lot of modifications. It seemed easier to just send a complete new version.
julia
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 21 lines · Julia Lawall
Quoted 19 lines · Stephen Boyd
Quoted 18 lines · Julia Lawall
Failure to terminate an
of_device_idtable can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.Cc Mitchel Humpherys <[email protected]>Cc Julia Lawall <[email protected]>Cc Gilles Muller <[email protected]>Cc Nicolas Palix <nicolas.palix-wMUr/[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>Signed-off-by Stephen Boyd <[email protected]>Acked-by Grant Likely <[email protected]>Signed-off-by Julia Lawall <[email protected]>---
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
v3: Removed unneeded rule
bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.These seem like only minor changes. Why was my authorship removed?
Probably because I don't know how to provide a new vesion of the patch properly. What should I have done?
If you use git format-patch it should insert a "From: <original author>" line at the beginning of the commit text. This ensures that when git am is used the correct authorship is retained. It looks like you're using alpine so I suspect you could just paste this information at the beginning of the message.
Signature · 7 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Julia Lawall
Acked-by Grant LikelyFrom: Stephen Boyd <[email protected]>
Failure to terminate an of_device_id table can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.
---
v3: Removed unneeded rule bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.
v4: corrected authorship
scripts/coccinelle/misc/of_table.cocci | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
scripts/coccinelle/misc/of_table.cocci new file +62 -0
@@ -0,0 +1,62 @@ +/// Make sure of_device_id tables are NULL terminated +// +// Keywords: of_table +// Confidence: Medium +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@depends on context@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, +* } +}; + +@depends on patch@ +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, +- } ++ }, ++ { } +}; + +@r depends on org || report@ +position p1; +identifier var, arr; +expression E; +@@ +struct of_device_id arr[] = { + ..., + { + .var = E, + } + @p1 +}; + +@script:python depends on org@ +p1 << r.p1; +arr << r.arr; +@@ + +cocci.print_main(arr,p1) + +@script:python depends on report@ +p1 << r.p1; +arr << r.arr; +@@ + +msg = "%s is not NULL terminated at line %s" % (arr, p1[0].line) +coccilib.report.print_report(p1[0],msg)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne 3.6.2014 20:25, Julia Lawall napsal(a):
Quoted 20 lines
From: Stephen Boyd <[email protected]>
Failure to terminate an
of_device_idtable can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.Cc Mitchel Humpherys <[email protected]>Cc Julia Lawall <[email protected]>Cc Gilles Muller <[email protected]>Cc Nicolas Palix <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>Signed-off-by Stephen Boyd <[email protected]>Acked-by Grant Likely <[email protected]>Signed-off-by Julia Lawall <[email protected]>---
Quoted 7 lines
v3: Removed unneeded rule
bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.v4: corrected authorship
scripts/coccinelle/misc/of_table.cocci | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
Applied to kbuild.git#misc, thanks. BTW, what is so special about of device drivers? Shouldn't we check for missing sentinels in all kinds of device tables?
Thanks,
Michal
Quoted 24 lines · Michal Marek
Dne 3.6.2014 20:25, Julia Lawall napsal(a):
Quoted 23 lines
From: Stephen Boyd <[email protected]>
Failure to terminate an
of_device_idtable can lead to confusing failures depending on where the compiler places the array. Add a check to make sure these tables are terminated. Thanks to Mitchel Humpherys for coming up with the pattern initially.Cc Mitchel Humpherys <[email protected]>Cc Julia Lawall <[email protected]>Cc Gilles Muller <[email protected]>Cc Nicolas Palix <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>Signed-off-by Stephen Boyd <[email protected]>Acked-by Grant Likely <[email protected]>Signed-off-by Julia Lawall <[email protected]>---
v3: Removed unneeded rule
bad_of_table. Placed * in context rule on the closing brace; putting it on the field caused all fields to be marked.
Quoted 8 lines
Quoted 4 lines
v4: corrected authorship
scripts/coccinelle/misc/of_table.cocci | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)Applied to kbuild.git#misc, thanks. BTW, what is so special about of device drivers? Shouldn't we check for missing sentinels in all kinds of device tables?
Yes the rule should probably be expanded to cover more tables. I guess this is just more noticeable because a driver may match some device that it isn't supposed to match and start writing random bits to registers it shouldn't be, causing the system to hang.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation